Make Alert Manager link to Treeherder rather than TBPL

RESOLVED FIXED

Status

Testing
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: emorley, Assigned: emorley)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
https://github.com/jmaher/alert_manager/blob/3558dee537d05526e745df60cc7a902c4f2dbed4/managed_settings.py#L2
https://github.com/jmaher/alert_manager/blob/d4a8b6c54f6d95c31b7cf574200659380155b68c/parse_news.py#L246

https://github.com/jmaher/alert_manager/search?utf8=%E2%9C%93&q=tbpl
(Assignee)

Updated

3 years ago
Blocks: 1029516
(Assignee)

Comment 1

3 years ago
Note the repo names are different to TBPL - they actually match the buildbot 'branch' now. See the "name" property in:
https://github.com/mozilla/treeherder-service/blob/master/treeherder/model/fixtures/repository.json
(Assignee)

Comment 2

3 years ago
Created attachment 8536250 [details] [review]
Link to Treeherder rather than TBPL

Apart from the branch TBPL referred to as "Firefox", all of the others listed in managed_settings.py just require a .lower() to match the 'real' repo/buildbot name that Treeherder uses.

I didn't go through and change all usages of 'tbpl' in variable/table names, since that would take ages and if one reads 'tbpl' as 'resultsdashboard' they still do make sense. The main thing is to make Alert Manager not link to TBPL, seeing as it will be going away soon.
Assignee: nobody → emorley
Status: NEW → ASSIGNED
Attachment #8536250 - Flags: review?(jmaher)
Comment on attachment 8536250 [details] [review]
Link to Treeherder rather than TBPL

r=me with the one fix mentioned on github and deployment will take place once treeherder has the popup pane available in production.
Attachment #8536250 - Flags: review?(jmaher) → review+
(Assignee)

Comment 4

3 years ago
The Talos panel is now in Treeherder production, can we merge this? :-)
merged and live.  Any new alerts coming in will get automatic treeherder links :)
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 6

3 years ago
Thanks for merging, sorry about the missing colon.

Re the param change, I'm pretty sure searchQuery is the correct param - and if it doesn't work, Treeherder needs fixing, I don't think this use case should be using filter-searchStr. There have been several regressions on Treeherder prod recently.

Cameron, can you confirm? (context: https://github.com/jmaher/alert_manager/commit/ee0d9e30e6ff1cb3725c87926a12aca8ae82228c)
Flags: needinfo?(cdawson)
thanks for the notes on this.  I just made things work- if searchQuery is what we should use, I will be happy to switch it back and help test as needed.
Hmm, sorry for the breakage on this.  I hadn't realized changing that param would have these implications.  I would *prefer* to keep the param as ``filter-searchStr`` or at least to keep the ``filter-`` prefix as it's part of the logic to distinguish filter params from other params.

However, if we have to make it work with ``searchQuery``, I could add a special case for it.  Is that worthwhile?  Or could we go forward with ``filter-searchStr``?
Flags: needinfo?(cdawson)
filter-searchStr is fine with me, it isn't very easy to remember, ideally it would be like:
https://treeherder.mozilla.org/ui/#/jobs?filter=linux talos dromaeo

if all usage of filter-searchStr are from programs then I don't see a problem with it.
You need to log in before you can comment on or make changes to this bug.