mozreview: Use BMO MozReview.attachments() API (
bug 1237491). r?smacleod
MozReview-Commit-ID: APK5D9iN9q3
--- a/pylib/mozreview/mozreview/bugzilla/client.py
+++ b/pylib/mozreview/mozreview/bugzilla/client.py
@@ -138,22 +138,21 @@ class BugzillaAttachmentUpdates(object):
flags.append({
'name': 'review',
'status': '?',
'requestee': r,
'new': True
})
if rb_attachment:
- params['ids'] = [rb_attachment['id']]
+ params['attachment_id'] = rb_attachment['id']
if rb_attachment['is_obsolete']:
params['is_obsolete'] = False
else:
- params['ids'] = [self.bug_id]
params['data'] = url
params['content_type'] = 'text/x-review-board-request'
params['file_name'] = 'reviewboard-%d-url.txt' % review_request_id
params['summary'] = "MozReview Request: %s" % summary
params['comment'] = comment
if flags:
params['flags'] = flags
@@ -164,41 +163,63 @@ class BugzillaAttachmentUpdates(object):
self.creates.append(params)
def obsolete_review_attachments(self, rb_url):
"""Mark any attachments for a given bug and review request as obsolete.
This is called when review requests are discarded or deleted. We don't
want to leave any lingering references in Bugzilla.
"""
- params = {'ids': [], 'is_obsolete': True}
-
self._update_attachments()
for a in self.attachments:
if (self.bugzilla._rb_attach_url_matches(a.get('data'), rb_url) and
not a.get('is_obsolete')):
- params['ids'].append(a['id'])
-
- if params['ids']:
- logger.info('Obsoleting attachments on bug %d: %s' % (
- self.bug_id, params['ids']))
- self.updates.append(params)
+ logger.info('Obsoleting attachment %s on bug %d:' % (
+ a['id'], self.bug_id))
+ self.updates.append({
+ 'attachment_id': a['id'],
+ 'is_obsolete': True
+ })
@xmlrpc_to_bugzilla_errors
def do_updates(self):
- for params in self.creates:
- self.bugzilla.proxy.Bug.add_attachment(
- self.bugzilla._auth_params(params))
- self.creates = self.creates[1:]
+ logger.info('Doing attachment updates for bug %s' % self.bug_id)
+ params = self.bugzilla._auth_params({
+ 'bug_id': self.bug_id,
+ 'attachments': self.creates + self.updates,
+ })
+
+ results = self.bugzilla.proxy.MozReview.attachments(params)
+
+ # The above Bugzilla call is wrapped in a single database transaction
+ # and should thus either succeed in creating and updating all
+ # attachments or will throw an exception and roll back all changes.
+ # However, just to be sure, we check the results. There's not much
+ # we can do in this case, but we'll log an error for investigative
+ # purposes.
+ # TODO: Display an error in the UI (no easy way to do this without
+ # failing the publish).
- for params in self.updates:
- self.bugzilla.proxy.Bug.update_attachment(
- self.bugzilla._auth_params(params))
- self.updates = self.updates[1:]
+ ids_to_update = set(u['attachment_id'] for u in self.updates)
+ ids_to_update.difference_update(results['attachments_modified'].keys())
+
+ if ids_to_update:
+ logger.error('Failed to update the following attachments: %s' %
+ ids_to_update)
+
+ num_to_create = len(self.creates)
+ num_created = len(results['attachments_created'])
+
+ if num_to_create != num_created:
+ logger.error('Tried to create %s attachments but %s reported as '
+ 'created.' % (num_to_create, num_created))
+
+ self.creates = []
+ self.updates = []
def _update_attachments(self):
if not self.attachments:
self.attachments = self.bugzilla.get_rb_attachments(self.bug_id)
class Bugzilla(object):
"""