1246831 - Error publishing: Bugzilla error: The flag type 'review' matches several flag types. You must specify the type id value to update or add a flag.

Status: RESOLVED INVALID

(MozReview Graveyard :: General, defect)

Description

[Gregory Szorc [:gps]]

eeejay reported an error publishing via IRC. Server logs follow.

2016-02-09 00:47:32,969 - ERROR -  - Error when calling <function on_review_request_publishing at 0x7fd10ef587d0> from SignalHook: Error publishing: Bugzilla error: The flag type 'review' matches several flag types. You must specify the type id value to update or add a flag.
Traceback (most recent call last):
  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 30, in _transform_errors
    raise PublishError('Bugzilla error: %s' % e.msg)
PublishError: Error publishing: Bugzilla error: The flag type 'review' matches several flag types. You must specify the type id value to update or add a flag.
2016-02-09 00:48:45,549 - ERROR -  - Error when calling <function on_review_request_publishing at 0x7fd10ed1d668> from SignalHook: Error publishing: Bugzilla error: The flag type 'review' matches several flag types. You must specify the type id value to update or add a flag.
Traceback (most recent call last):
  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 30, in _transform_errors
    raise PublishError('Bugzilla error: %s' % e.msg)
PublishError: Error publishing: Bugzilla error: The flag type 'review' matches several flag types. You must specify the type id value to update or add a flag.

Comment 1

[Gregory Szorc [:gps]]

This is publishing to bug 1166365. Currently in that bug, a MozReview text/x-review-board-request attachment is marked as obsolete. I wonder if that triggers it.

Comment 2

[Eitan Isaacson [:eeejay]]

I ran into it before obsoleting it, and also after.

Comment 3

[Gregory Szorc [:gps]]

glob: I reckon we are calling the Bugzilla API incorrectly? That being said, I'm not sure how "review" could be ambiguous on the Bugzilla side of things...

Comment 4

[:glob ✱]

(In reply to Gregory Szorc [:gps] from comment #3)
> That being said, I'm not sure how "review" could be ambiguous on the Bugzilla side of
> things...

bmo current has five flags called 'review', with different configurations depending on the product.
normally only one of these review flags are visible at a time and is unambiguous.

bug 1166365 is special.  it's in the 'graveyard'.  products/components in the graveyard are defunct.

in order to move bugs without dropping data, applicable flags are made visible in the graveyard before a component is moved.  this leaves open the possibility for two 'review' flags to be visible at the same time, if the product used different flags over the course of its life.

you can see this on https://bugzilla.mozilla.org/attachment.cgi?id=8693935&action=edit - there's two review flags (flagtype-4 and flagtype-748).


tricky.

we can't disable either of those flags for many reasons (flags visibility is per-product/component, but its is_active bit is singular), and even if we could that would still throw an error back to mozreview because we wouldn't be able to set the flag.

i've filed bug 1246859 to address clearing of review flags prior to graveyarding.

> I reckon we are calling the Bugzilla API incorrectly?

not really.

in this case i don't see any review flags that needed to be cleared; it looks like mozreview asks bugzilla to clear the review flag, which would normally be a no-op if it wasn't set, but in this case bugzilla couldn't determine that due to the ambiguous request.

we could put special handling into the bugzilla side of mozreview's batch request endpoint that handles requests for products in the graveyard classifications slightly differently:
- when setting, throw an error that clearly states that the product has been graveyarded
- when clearing, clear all possible review flags instead of throwing an error

gps, does that sound sane?

Comment 5

[Gregory Szorc [:gps]]

(In reply to Byron Jones ‹:glob› from comment #4)
> we could put special handling into the bugzilla side of mozreview's batch
> request endpoint that handles requests for products in the graveyard
> classifications slightly differently:
> - when setting, throw an error that clearly states that the product has been
> graveyarded
> - when clearing, clear all possible review flags instead of throwing an error

That sounds sane to me. I do worry about performance implications - the excessive interaction between Review Board and Bugzilla is already a major source of slowdown when submitting commits to MozReview. Although without looking at the code, I /think/ we already perform a lookup on the bug to get basic info, so perhaps no new HTTP request overhead will be introduced here.

Comment 6

[:glob ✱]

(In reply to Gregory Szorc [:gps] from comment #5)
> (In reply to Byron Jones ‹:glob› from comment #4)
> > we could put special handling into the bugzilla side of mozreview's batch
> > request endpoint that handles requests for products in the graveyard
> > classifications slightly differently:
> > - when setting, throw an error that clearly states that the product has been
> > graveyarded
> > - when clearing, clear all possible review flags instead of throwing an error
> 
> That sounds sane to me. I do worry about performance implications - the
> excessive interaction between Review Board and Bugzilla is already a major
> source of slowdown when submitting commits to MozReview. Although without
> looking at the code, I /think/ we already perform a lookup on the bug to get
> basic info, so perhaps no new HTTP request overhead will be introduced here.

this wouldn't introduce any new lookups or measurable performance overheads .. we'll put the graveyard handling into the bugzilla endpoint, not require mozreview to determine a bug's classification and flag state.

ie. mozreview will continue to ask bugzilla to 'clear the review flag', and if it's for a bug in the graveyard things will 'just work' instead of throwing an error about multiple matching flags.

filed as bug 1246861.

Comment 7

[BMO Automation]

MozReview is now obsolete. Please use Phabricator instead. Closing this bug.