WOO needs to be branch-aware

RESOLVED FIXED

Status

Tree Management
OrangeFactor
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: jgriffin, Unassigned)

Tracking

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

7 years ago
Changes to the woo logparser are going live soon, which will cause data from multiple trees to be added to ES.  However, OF will not show them at present, since most of the ES queries in woo_server.py are hard-coded to use only mozilla-central.  

Additionally, most of the UI is not branch-aware.  We probably need to add a tree chooser to every view (maybe part of the top-right nav bar?) that allows the user to choose a tree (default: mozilla-central), and this info will be sent with requests to woo_server.py so that the queries will return data from the selected tree.

Current trees that are enabled in ES:  mozilla-central, places, jaegermonkey, mozilla-aurora, mozilla-beta
(Reporter)

Comment 1

7 years ago
Created attachment 527128 [details] [diff] [review]
woo_server.py patch

This patch adds an optional tree parameter to all the woo_server api's which are not tree-agnostic, and defaults to mozilla-central if not specified.  I don't think there is any use in returning data for all branches in any of our views, but if there is, I can modify the patch.
Attachment #527128 - Flags: review?(mcote)

Comment 2

7 years ago
We should probably have a mode which presents all of the information in a single view (like OrangeFactor does today).

This is useful to determine most highly occurring oranges, for example.
(Reporter)

Comment 3

7 years ago
Created attachment 527174 [details] [diff] [review]
woo_server.py patch v0.2

Updated patch which defaults to returning data for all trees.
Attachment #527128 - Attachment is obsolete: true
Attachment #527128 - Flags: review?(mcote)
Attachment #527174 - Flags: review?(mcote)

Comment 4

7 years ago
Comment on attachment 527174 [details] [diff] [review]
woo_server.py patch v0.2

Looks good.

Btw I had to do an emergency checkin (http://hg.mozilla.org/automation/orangefactor/rev/97450fcec4f1) to add 'jetpack' to the list of testruns because BugCount was broken when displaying tests by testrun.
Attachment #527174 - Flags: review?(mcote) → review+
(Reporter)

Comment 5

7 years ago
Created attachment 527674 [details] [diff] [review]
woo_server.py patch v0.3

This version fixes the problem of using too much of the builder string as the platform string for trees that are subtrees, e.g., projects/*
Attachment #527174 - Attachment is obsolete: true
(Reporter)

Comment 6

7 years ago
Created attachment 527679 [details] [diff] [review]
woo_server.py patch v0.4

Fix final problem with subtrees, +rebase
Attachment #527674 - Attachment is obsolete: true
(Reporter)

Comment 7

7 years ago
Created attachment 527682 [details] [diff] [review]
woo_server.py patch v0.5

Another change; this fixes an issue with the raw JSON for testfailures that arose for data which was parsed using the new logparser.
(Reporter)

Comment 8

7 years ago
Created attachment 528243 [details] [diff] [review]
woo_server.py patch v0.6

This version returns data only for the following trees, when no tree is specified:  'mozilla-central', 'mozilla-aurora', 'mozilla-beta', 'cedar', 'places'

It also uses the tbpl platform (one of: windows, windowsxp, windows7-64, osx, osx64, linux, linux64) instead of trying to extract the platform from the builder string; the latter is hard since the builder string is formatted very inconsistently across branches.
Attachment #527679 - Attachment is obsolete: true
Attachment #527682 - Attachment is obsolete: true
Attachment #528243 - Flags: review?(mcote)

Comment 9

7 years ago
Comment on attachment 528243 [details] [diff] [review]
woo_server.py patch v0.6

Good as before, although I noticed one thing:

+    # if a 'tree' argument exists, add it to the querydict
+    def addTreeArgument(self, querydict, args):
+        querydict.update({ 'tree': args.get('tree', self.alltrees) })
+

Just to be clear--we expect that the 'tree' argument will either be one single tree name or be blank/missing, correct?  I noticed that nothing parses a list of trees.  It isn't possible in the UI currently to select more than one tree anyway, but it just strikes me as a bit weird that if an argument is provided, it has to be a single string, but if one isn't provided, we default to a tuple.
Attachment #528243 - Flags: review?(mcote) → review+
Created attachment 528251 [details] [diff] [review]
UI changes to enable tree selection

Here are the necessary UI changes to select a tree (or all).

I tried to make it fairly generic, in case we ever have more controls like this one.  This is a "global" control, meaning the selection is preserved across views, unlike the other controls (filters and such).  So if you choose mozilla-central and go to the details for a bug, click on a graph, navigate to the Simulator via the top-right menu, etc., you will still be looking at the mozilla-central tree.

After reviewing this, we should sync up to commit our two patches at approximately the same time and then deploy them both.
Attachment #528251 - Flags: review?(jgriffin)
(Reporter)

Comment 11

7 years ago
(In reply to comment #9)
> Comment on attachment 528243 [details] [diff] [review]
> woo_server.py patch v0.6
> 
> Good as before, although I noticed one thing:
> 
> +    # if a 'tree' argument exists, add it to the querydict
> +    def addTreeArgument(self, querydict, args):
> +        querydict.update({ 'tree': args.get('tree', self.alltrees) })
> +
> 
> Just to be clear--we expect that the 'tree' argument will either be one single
> tree name or be blank/missing, correct?  I noticed that nothing parses a list
> of trees.  It isn't possible in the UI currently to select more than one tree
> anyway, but it just strikes me as a bit weird that if an argument is provided,
> it has to be a single string, but if one isn't provided, we default to a tuple.

Yes, that's correct.  The reason we default to a tuple is to prevent the API's from returning data for trees we don't care about (e.g., try), but for which we have some data, when no tree is explicitly specified.

This argument gets passed directly to the ES API's, which support either a single string arg or a tuple, depending on whether or not you want to query against single or multiple values.

If this seems too weird, maybe I need to update the ES API's.
Ah all I meant is that maybe we should check if the user argument is a comma-separated list, in which case we split it and send it to ES.  It's not weird that it takes both a string and a tuple (that kind of thing is pretty Pythonic) but rather that a user argument can only be a single string.  But it's not very important, since as I mentioned the UI doesn't (currently) allow multiple trees.
(Reporter)

Comment 13

7 years ago
Comment on attachment 528251 [details] [diff] [review]
UI changes to enable tree selection

Review of attachment 528251 [details] [diff] [review]:

Looks great!
Attachment #528251 - Flags: review?(jgriffin) → review+
(Reporter)

Comment 14

7 years ago
(In reply to comment #12)
> Ah all I meant is that maybe we should check if the user argument is a
> comma-separated list, in which case we split it and send it to ES.

Makes sense, I made this changed and pushed as http://hg.mozilla.org/automation/orangefactor/rev/543d3ee15e98
Done here.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
Product: Testing → Tree Management
You need to log in before you can comment on or make changes to this bug.