Closed Bug 799637 Opened 9 years ago Closed 9 years ago

AnnotatedSummaryGenerator.php's BzAPI orange bug search should specify the exact columns required

Categories

(Tree Management Graveyard :: TBPL, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: emorley, Assigned: emorley)

References

Details

Attachments

(1 file, 1 obsolete file)

If columns aren't specified when making a bzapi request, you get the default 150 fields, which requires more joins on the b.m.o DB side & means a larger json response.

At most we need:
{bug number, summary, status, resolution, last changed}

(and in some places, less)

Need to change:
* https://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/file/9c7f496beb97/php/inc/AnnotatedSummaryGenerator.php#l100
* https://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/file/9c7f496beb97/js/Data.js#l28

(And if bug 799603 were to end up being wontfix:
* https://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/file/9c7f496beb97/php/submitBugzillaComment.php#l32 )
Depends on: 800646
Depends on: 800652
Depends on: 800875
Attached patch WIP (obsolete) — Splinter Review
The AnnotatedSummaryGenerator.php change has uncovered a few BzAPI oddities when using include_fields with 'resolution', as such I've broken the UI part (link #2 in comment 0) out to bug 800875, so it's not blocked on them.

Link #3 in comment 0 is no longer relevant, as that block has been removed.

This just leaves the AnnotatedSummaryGenerator.php change, for which I'm check-pointing a WIP patch.

It:
* Adds "&include_fields=id,summary,status,resolution"
* Removes the (I believe to be) unnecessary mapping in the array_map function - which I can only think was added to strip out the unwanted fields, to avoid sticking them all in the bugscache.
* Adds the resolution field if we didn't get it, to work around bug 800646.

TODO:
* Pending the result of bug 800652, add our own sorting of the bugs array, so the bug suggestions shown in the annotated summary are in the same order as they are currently (or at least have open bugs at the top).

--

In the future, I want to replace the bugscache with a proper cache of all filed orange bugs, which will make this bzapi call redundant. Combined with my desire for us to eventually split out closed bug suggestions in the annotated summary (and put under a "show more", which means the sort order won't matter); it's quite a pain this wasn't just a simple case of adding include_fields as a stop-gap - since we'll be replacing much of this soon :-(
Oh and I forgot the other fun part:

Since we no longer time out as much in the bzapi call, we now return ridiculous number of bug suggestions for certain failure types (that we previously wouldn't have given any suggestions for).

eg "Shutdown"

https://api-dev.bugzilla.mozilla.org/latest/bug?whiteboard=orange&include_fields=id,summary,status,resolution&summary=Shutdown

So I'll need to figure out what else to add to the ignore filter.
Blocks: 790889
Depends on: 800888
No longer depends on: 800875
Summary: TBPL's bzapi requests should specify the exact columns required (not use the default of 150 fields) → AnnotatedSummaryGenerator.php's BzAPI orange bug search should specify the exact columns required
Attached patch Patch v1Splinter Review
* Now that bug 800646 and bug 800652 are fixed, the returned JSON no longer differs when include_fields is used, so the workarounds mentioned previously are not needed.
* Now that we don't get a load of unused fields, we don't need to do the array_map() dance to filter them out :-)
Attachment #670767 - Attachment is obsolete: true
Attachment #672261 - Flags: review?(arpad.borsos)
Attachment #672261 - Flags: review?(arpad.borsos) → review+
Looking at tbpl-dev import logs, this appears to have reduced bug lookup times from ~4400-4700ms to ~2300-2700ms - pretty happy with this :-)
Depends on: 804021
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: Webtools → Tree Management
Product: Tree Management → Tree Management Graveyard
You need to log in before you can comment on or make changes to this bug.