bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Exception thrown when carrying forward r- on publish

RESOLVED FIXED

Status

MozReview
Integration: Bugzilla
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mcote, Assigned: mcote)

Tracking

Production

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
ktbee reported a 500 error when trying to publish a review request.  This is the exception:

- Exception thrown for user kbroida+567154 at https://reviewboard.mozilla.org/api/review-requests/58298/draft/

'jaws@mozilla.com'
Traceback (most recent call last):
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/django/core/handlers/base.py", line 112, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/lib64/python2.6/site-packages/newrelic-2.44.0.36/newrelic/hooks/framework_django.py", line 497, in wrapper
    return wrapped(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/django/views/decorators/cache.py", line 52, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/django/views/decorators/vary.py", line 19, in inner_func
    response = func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/resources/base.py", line 196, in __call__
    request, method, view, api_format=api_format, *args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/resources/mixins/api_tokens.py", line 65, in call_method_view
    return view(request, *args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/resources/base.py", line 464, in put
    return self.update(request, *args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/reviewboard/webapi/decorators.py", line 139, in _check
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 143, in _checklogin
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 307, in _validate
    return view_func(*args, **new_kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/reviewboard/webapi/resources/review_request_draft.py", line 493, in update
    review_request.publish(user=request.user, trivial=trivial)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/reviewboard/reviews/models/review_request.py", line 809, in publish
    review_request_draft=draft)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/django/dispatch/dispatcher.py", line 185, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/extensions/hooks.py", line 196, in _wrap_callback
    self.callback(extension=self.extension, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/bugzilla/errors.py", line 28, in _transform_errors
    return func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/signal_handlers.py", line 314, in on_review_request_publishing
    children_to_obsolete)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/bugzilla/attachments.py", line 130, in update_bugzilla_attachments
    carry_forward)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/bugzilla/client.py", line 112, in create_or_update_attachment
    if not carry_forward[f['setter']]:
KeyError: 'jaws@mozilla.com'
(Assignee)

Comment 1

2 years ago
Actually, ktbee got an r- in bug 565718 comment 13, and there was no r+.
(Assignee)

Updated

2 years ago
Summary: Exception thrown when carrying forward r+ on publish → Exception thrown when carrying forward r- on publish
(Assignee)

Comment 2

2 years ago
Other possibly salient points:

* After their first commit, the author had issues with their local clone and ended up blowing away their clone, recloning, and then creating a new commit that addressed the review points.  I think this should be fine, though, and certainly shouldn't result in a 500.
* The new commit (intentionally) changed reviewers by switching from r?jaws to r?dao in the commit message.
(Assignee)

Comment 3

2 years ago
Created attachment 8764088 [details]
mozreview: Fix erroneous assumption that an r- will always be in the carry_forward dict (bug 1281273);

BugzillaAttachmentUpdates.create_or_update_attachment() presumed
that a reviewer who left an r- will always be in the carry_forward
dict, which is not the case if they have not been requested for
re-review.

In general this logic is more confusing than it needs to be, so
I also added a big FIXME.

Review commit: https://reviewboard.mozilla.org/r/60134/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60134/
Attachment #8764088 - Flags: review?(glob)
(Assignee)

Comment 4

2 years ago
https://reviewboard.mozilla.org/r/60134/#review57010

I realize I should add a test for this case.  I'll do that tomorrow, but in the meantime if you're around, you can review the code.
Assignee: nobody → mcote
Attachment #8764088 - Flags: review?(glob) → review+
Comment on attachment 8764088 [details]
mozreview: Fix erroneous assumption that an r- will always be in the carry_forward dict (bug 1281273);

https://reviewboard.mozilla.org/r/60134/#review57022

lgtm
(Assignee)

Comment 6

2 years ago
Created attachment 8764441 [details]
tests: Test the case in which reviewers are switched after leaving reviews (bug 1281273);

Review commit: https://reviewboard.mozilla.org/r/60328/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60328/
Attachment #8764441 - Flags: review?(glob)
(Assignee)

Comment 7

2 years ago
Comment on attachment 8764088 [details]
mozreview: Fix erroneous assumption that an r- will always be in the carry_forward dict (bug 1281273);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60134/diff/1-2/
(Assignee)

Comment 8

2 years ago
Comment on attachment 8764441 [details]
tests: Test the case in which reviewers are switched after leaving reviews (bug 1281273);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60328/diff/1-2/
(Assignee)

Comment 9

2 years ago
Comment on attachment 8764441 [details]
tests: Test the case in which reviewers are switched after leaving reviews (bug 1281273);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60328/diff/2-3/
Attachment #8764441 - Flags: review?(glob) → review+
Comment on attachment 8764441 [details]
tests: Test the case in which reviewers are switched after leaving reviews (bug 1281273);

https://reviewboard.mozilla.org/r/60328/#review57192

lgtm

Comment 11

2 years ago
Pushed by mcote@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/83c138d64dd9
mozreview: Fix erroneous assumption that an r- will always be in the carry_forward dict ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/b7d62b373add
tests: Test the case in which reviewers are switched after leaving reviews ; r=glob
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.