1243021 - bugzilla integration throws internal server error when a child review request is discarded

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect)

Description

[Axel Hecht [:Pike]]

Something's funky with https://reviewboard.mozilla.org/r/31033/ and it's integration into bug 721211.

Comment 1

[:glob ✱]

https://reviewboard.mozilla.org/api/extensions/mozreview.extension.MozReviewExtension/summary/?bug=721211 is throwing an ISE.

Comment 2

[:glob ✱]

looks like:

File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/resources/review_request_summary.py", line 282, in _summarize_families for child_rrid in child_rrids
KeyError: 31033

there's quite a few requests that are also showing the same issue:

more examples:

https://reviewboard.mozilla.org/r/28733/
https://reviewboard.mozilla.org/api/extensions/mozreview.extension.MozReviewExtension/summary/?bug=1208371

https://reviewboard.mozilla.org/r/22965/
https://reviewboard.mozilla.org/api/extensions/mozreview.extension.MozReviewExtension/summary/?bug=1205843

https://reviewboard.mozilla.org/r/28729/
https://reviewboard.mozilla.org/api/extensions/mozreview.extension.MozReviewExtension/summary/?bug=1208371

Comment 3

[:glob ✱]

looks like all of those requests have been discarded.

Comment 4

[Axel Hecht [:Pike]]

I got into that state by trying to actually publish the updated diff of a formerly discarded review request.

I broke try, so that's why I needed to cancel the review requests.

Comment 5

[Axel Hecht [:Pike]]

Is there anything that you or I could do to actually unblock the work in that bug of mine? Short of not using mozreview for at least that bug?

Comment 6

[:glob ✱]

sorry about the delays in responding.  i've been looking through the code and logs for this without much luck, and the relevant parts of the code are in a state of flux right now.

as far as i can tell this error shouldn't break functionality within mozreview itself.  i'll check back once everything's been re-enabled again after this week's work.

Comment 7

[:glob ✱]

looks like what's going on here is discarded review requests are skipped in _sort_families (line 180), however _summarize_families reads all commits from the parent's p2rb.commits (line 277) and expects them to be present in the sorted family list (line 286).

we should either return discarded requests to bmo, or apply the same filtering in _summarize_families().

note currently review board doesn't render discarded requests any differently in its commits table; i'll chat with the team to see what the expected behavour is there.

Comment 8

[:glob ✱]

discarded review requests should be ignored everywhere - bug 1248425

Comment 9

[:glob ✱]

Attachment: MozReview Request: mozreview: discarded child reviews shouldn't break bmo integration (bug 1243021) r?mdoglio

Fixes a crash triggered by a child review request being marked as discarded.

Review commit: https://reviewboard.mozilla.org/r/34985/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34985/

Comment 10

[Mauro Doglio [:mdoglio]]

Comment on attachment 8719527 [details]
MozReview Request: mozreview: discarded child reviews shouldn't break bmo integration (bug 1243021) r?mdoglio

https://reviewboard.mozilla.org/r/34985/#review31549

::: pylib/mozreview/mozreview/resources/review_request_summary.py:291
(Diff revision 1)
> -                    for child_rrid in child_rrids
> +                    for child_rrid in filter(

to filter a list comprehension it's probably more idiomatic to use a `if` clause at the end of it.

```python
for child_rrid in child_rrids if child_rrid in family['children']
```

Comment 11

[:glob ✱]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/79f7e3cb5409