MozReview reviewer deduction not working

RESOLVED DUPLICATE of bug 1177567

Status

defect
P1
normal
RESOLVED DUPLICATE of bug 1177567
4 years ago
3 years ago

People

(Reporter: botond, Unassigned)

Tracking

Details

Attachments

(1 attachment)

STR:
  Push a review request containing a commit whose commit message
  contains the string "r=tn".

Expected results:
  "tn" (Timothy Nikkel) gets automatically populated in the reviewer
   field for the commit.

Actual results:
  The reviewer field stays blank.

Note that I have the latest version of version-control-tools, and I get the 

  "(It appears you are using r= to specify reviewers for a patch under review. 
    Please use r? to avoid ambiguity as to whether or not review has been granted.)" 

warning message, as expected.

It doesn't work with "r=kats" either, but there the problem might be related to bug 1139662, because entering "kats" manually triggers an internal server error (as described in that bug). Entering "tn" manually, however, works fine, so this seems to be a  different bug.
I had r?gps fail for me as well. :mcote was able to use it successfully when we pushed it to production, so I'm not sure if it is failing for some reviewers and not others, or if something has subsequently gone wrong.

Fixing Bug 1164127 to give feedback on unrecognized reviewers should help identify what's going wrong.
Priority: -- → P2
I had a space before the 'r' in my commit messages.
Duplicate of this bug: 1170374
I'm also seeing this, for r?ally .
Not working with "r=ehsan" either.
Priority: P2 → P1
I got the warning message on a review push today:
```
unrecognized reviewer: jrmuizel

changeset:  250793:d143c2f403fd
summary:    bug 1174415 - Make gfx build for iOS. r?jrmuizel
review:     https://reviewboard.mozilla.org/r/11357 (draft)
```

Going to the review and putting that same nick in the reviewer field works, however.
I broke out the relevant portion of code [1] into a standalone command line utility. It seems to work fine against both reviewboard and reviewboard-dev for reviewers that fail for me when I push to mozreview.

I think the next step is to log the actual exception at [2] to see if we're getting more than just APIErrors due to bad usernames.

[1] https://dxr.mozilla.org/hgcustom:version-control-tools/source/pylib/reviewboardmods/reviewboardmods/pushhooks.py#166

[2] https://dxr.mozilla.org/hgcustom:version-control-tools/source/pylib/reviewboardmods/reviewboardmods/pushhooks.py#201
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #7)
> I got the warning message on a review push today:
> unrecognized reviewer: jrmuizel
>
> [...]
> 
> Going to the review and putting that same nick in the reviewer field works,
> however.

Likewise, for 'kats'.
(In reply to Dan Minor [:dminor] from comment #8)
> I broke out the relevant portion of code [1] into a standalone command line
> utility. It seems to work fine against both reviewboard and reviewboard-dev
> for reviewers that fail for me when I push to mozreview.
> 
> I think the next step is to log the actual exception at [2] to see if we're
> getting more than just APIErrors due to bad usernames.

This logging was added and deployed to the dev instance. I tried pushing there with "r=kats", and got:

error identifying reviewer: kats: HTTP 500
Posted file xmlresponse
We're breaking because the xml rpc we're making gets a response that the expat parser can't parse. The character which explodes things is '\x1b'.

I've attached the body of the xml response bugzilla gives back.

The user that appears to break us is https://bugzilla.mozilla.org/user_profile?login=yokada%40art.osaka-med.ac.jp due to their full name.
Depends on: 1176452
Filed bug 1176452 with example script.  I set it to block this bug, but we can remove that if we want to work around it somehow.
(In reply to Botond Ballo [:botond] from comment #10)
> (In reply to Dan Minor [:dminor] from comment #8)
> > I broke out the relevant portion of code [1] into a standalone command line
> > utility. It seems to work fine against both reviewboard and reviewboard-dev
> > for reviewers that fail for me when I push to mozreview.
> > 
> > I think the next step is to log the actual exception at [2] to see if we're
> > getting more than just APIErrors due to bad usernames.
> 
> This logging was added and deployed to the dev instance. I tried pushing
> there with "r=kats", and got:
> 
> error identifying reviewer: kats: HTTP 500

Today I did a different push with "r=karlt", and got a different error:

error identifying reviewer: karlt: Bugzilla error: Logged-out users cannot use the "match" argument to this function to access any user information. (HTTP 500, API Error 226)

Is that a different issue?
(In reply to Botond Ballo [:botond] from comment #13) 
> Today I did a different push with "r=karlt", and got a different error:
> 
> error identifying reviewer: karlt: Bugzilla error: Logged-out users cannot
> use the "match" argument to this function to access any user information.
> (HTTP 500, API Error 226)
> 
> Is that a different issue?

Yes, it appears we have another bug here. We finally deployed some new error reporting to the hg server and this showed up.
Duplicate of this bug: 1139662
I split the auth problem into another bug: bug 1177454.
Apparently fixing the bug in Bugzilla is nontrivial, and XMLRPC is deprecated now anyway.  I'm going to dupe this bug over to bug 1177567, where we'll be moving to Bugzilla's REST API.

In the meantime, we've edited the problematic user's real name to strip out the escape character, since the user has never done anything with BMO after signing up.  I verified that r?kats now correctly sets him as reviewer.  If anyone runs into more problematic user name before we switch to REST, you can comment here and we'll try to sanitize them as well.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1177567
I can confirm that reviewer deduction is now working for me (I've tried with 'r=kats' and 'r=tn'). Thanks for the investigation and fix!
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.