calls for resultset jobs doesn't honor ``visibility`` param

RESOLVED FIXED

Status

Tree Management
Treeherder
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: camd, Assigned: camd)

Tracking

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
There is a URL param called ``visibility`` that can be passed in to decide what to show the user wrt the current exclusion_profile.  But we do not pass that along to the calls to the /jobs endpoint so the feature is broken.  visibility defaults to "included" to only show you jobs that should be included for the given exclusion_profile.

What SHOULD happen is that you specify a URL like:
treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&exclusion_profile=Test&visibility=excluded

And this will show you all the jobs that WOULD be excluded by the exclusion profile called "Test"
(Assignee)

Updated

2 years ago
Assignee: nobody → cdawson

Comment 1

2 years ago
Created attachment 8741165 [details] [review]
[treeherder] mozilla:visibility-param-for-jobs > mozilla:master
(Assignee)

Comment 2

2 years ago
Hey Wes, here's that fix I was talking about.  Feel free to give it a try from a local instance.
Flags: needinfo?(wkocher)
(Assignee)

Comment 3

2 years ago
This will make it possible (easy?) for Sheriffs to see exactly what's being affected by a profile.  Should be handy.  :)
(Assignee)

Comment 4

2 years ago
Comment on attachment 8741165 [details] [review]
[treeherder] mozilla:visibility-param-for-jobs > mozilla:master

I've had sheriffs ask questions around this subject a few times.  And the functionality was ALMOST completely there.  On the django side, it pretty much was.  An hour or so's work got me this, and I thought it'd be useful.
Attachment #8741165 - Flags: review?(wlachance)
Comment on attachment 8741165 [details] [review]
[treeherder] mozilla:visibility-param-for-jobs > mozilla:master

This looks great, would it be possible to do up a quick unit test for the case that you fixed in jobs endpoint? I don't think it should be too hard...
Attachment #8741165 - Flags: review?(wlachance) → review-
Comment on attachment 8741165 [details] [review]
[treeherder] mozilla:visibility-param-for-jobs > mozilla:master

This works pretty good. All I could ask for beyond this is to be able to show individual exclusions, not just profiles.

One thing to note is that the "Tiers" filters are still in effect. Eg, if I do http://local.treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&exclusion_profile=Tier-3&visibility=excluded (to show the tier-3 profile, Treeherder won't show anything until I check the box next to "Tier-3" in the Tiers menu. Unsure if that's a big deal or even if it's wrong.
Flags: needinfo?(wkocher)
Attachment #8741165 - Flags: feedback+
(Assignee)

Comment 7

2 years ago
(In reply to Wes Kocher (:KWierso) from comment #6)
> Comment on attachment 8741165 [details] [review]
> [treeherder] mozilla:visibility-param-for-jobs > mozilla:master
> 
> This works pretty good. All I could ask for beyond this is to be able to
> show individual exclusions, not just profiles.

Yeah, we could add that at some point.  You can always create a new profile that has just your JobExclusion in it to get that effect.

> 
> One thing to note is that the "Tiers" filters are still in effect. Eg, if I
> do
> http://local.treeherder.mozilla.org/#/jobs?repo=mozilla-
> inbound&exclusion_profile=Tier-3&visibility=excluded (to show the tier-3
> profile, Treeherder won't show anything until I check the box next to
> "Tier-3" in the Tiers menu. Unsure if that's a big deal or even if it's
> wrong.

Yeah, I noticed that, too.  I'm not sure what the right answer is unless I maybe make those "show excluded" links add all tiers to the URL.  I kind of think the current action is correct, but folks could get mislead by that, too.  Maybe we could add that in later if it seems to be a problem.
(Assignee)

Comment 8

2 years ago
Comment on attachment 8741165 [details] [review]
[treeherder] mozilla:visibility-param-for-jobs > mozilla:master

Good point.  I added a unit test for that.  :)
Attachment #8741165 - Flags: review- → review?(wlachance)
Comment on attachment 8741165 [details] [review]
[treeherder] mozilla:visibility-param-for-jobs > mozilla:master

r+ with the nits re: the unit test in the review fixed. :)
Attachment #8741165 - Flags: review?(wlachance) → review+

Comment 10

2 years ago
Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/a0725b61cf22cee3dff00aa669ebbb20fbcec580
Bug 1264495 - honor visibility url param to see what jobs are excluded

Passes the ``exclusion_profile`` and ``visibility`` url params from
the main treeherder URL into the calls for jobs in each resultset.
With this, you can see exactly what will jobs are affected by the given
profile.

This also adds a button in the sheriff panel to quickly add these params
for each exclusion profile and open that in a new tab.

Also required a tiny fix for passing in an exclusion profile on a
repo that is not mentioned in that profile, but also passing in visibility
excluded.  It was just returning ALL jobs, where it should have
returned no jobs.
(Assignee)

Comment 11

2 years ago
nits fixed and merged.  Thanks wlach!
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.