Add support for l10n repacks to Treeherder

RESOLVED FIXED

Status

P1
major
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: RyanVM, Assigned: camd)

Tracking

Details

Attachments

(1 attachment)

54 bytes, text/x-github-pull-request
emorley
: review+
RyanVM
: review+
RyanVM
: feedback+
Details | Review | Splinter Review
(Reporter)

Description

4 years ago
These jobs were recently added and are showing up as unknown on Treeherder.

Firefox mozilla-central linux nightly l10n x/3
Firefox mozilla-central linux64 nightly l10n x/3
Firefox mozilla-central macosx64 nightly l10n x/3

The linux jobs show up as regular nightlies. The OSX ones are completely unknown. I'm not even sure how I'm going to hide these at the moment.
(Reporter)

Comment 1

4 years ago
Firefox mozilla-central win32 nightly l10n x/3
(Reporter)

Comment 2

4 years ago
Firefox mozilla-central win64 nightly l10n x/3
(Reporter)

Comment 3

4 years ago
I was hoping to get a patch up for this, but I'm out of time for the day. I'd envision these being handled similar to how we deal with Android, i.e. L10N(N1 N2 N3).

Cam, given that this is aggravated by bug 1087349, can we please prioritize fixing and getting this into production today?
Flags: needinfo?(cdawson)
Priority: -- → P1
(Assignee)

Comment 4

4 years ago
working on a patch for this now.
Flags: needinfo?(cdawson)
(Assignee)

Updated

4 years ago
Assignee: nobody → cdawson
(Assignee)

Comment 5

4 years ago
Created attachment 8588181 [details] [review]
l10n repack support PR
Attachment #8588181 - Flags: review?(mdoglio)
Attachment #8588181 - Flags: feedback?(ryanvm)
(Reporter)

Comment 6

4 years ago
Comment on attachment 8588181 [details] [review]
l10n repack support PR

Thanks for jumping on this :)
Attachment #8588181 - Flags: feedback?(ryanvm) → feedback+
(Assignee)

Comment 7

4 years ago
Ryan- If you'd like me to try to push this out today, perhaps you could do the review to see if it looks right to you?  Ed and Mauro are both out today.
Flags: needinfo?(ryanvm)
(Reporter)

Comment 8

4 years ago
Comment on attachment 8588181 [details] [review]
l10n repack support PR

Given the passing tests and  in the interests of getting this into production ASAP, r=me with the win64 change to buildbot.py moved up to the pre-existing win64 block as discussed on IRC. That said, I'd still like Mauro/Ed to take a retroactive look at this since my regex-fu isn't exactly superb :)
Flags: needinfo?(ryanvm)
Attachment #8588181 - Flags: review+
(Assignee)

Updated

4 years ago
Attachment #8588181 - Flags: review?(mdoglio) → review?(emorley)
(Assignee)

Comment 9

4 years ago
Ed: I hope it's ok to transfer the review of this to you, since you often review stuff like this.
Status: NEW → ASSIGNED
Comment on attachment 8588181 [details] [review]
l10n repack support PR

IMO we should insist they fix the buildernames; some time ago I spent a while getting rid of the old style "X/Y" numbering and simplifying all the regex as a result; and they've now regressed this. 

Things like:
NUMBER_RE = re.compile(r"((?<=-)\d+|\d+(?=/\d+))(?:/\d+)?$")

...just make me want to stab my eyes out :-(

I've added a couple of tweaks to the PR; and will file a bug to get them to fix the buildernames, and then we can remove the support for the alternative format.
Attachment #8588181 - Flags: review?(emorley) → review+

Updated

4 years ago
Depends on: 740142
To be clear: I didn't mean this was your fault Cameron, you weren't too know the history here, it's just been a frustrating few years of back and forth with tbpl and buildbot-config changes that are often inconsistent and we're only told about retrospectively. One day... :-)
(Assignee)

Comment 13

4 years ago
No worries at all.  Sounds good.  Task Cluster to the rescue!  :)
(Assignee)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(In reply to Cameron Dawson [:camd] from comment #13)
> Task Cluster to the rescue!  :)

Indeed! Thanks for taking this bug whilst I was away :-)

Updated

4 years ago
Blocks: 1153138
You need to log in before you can comment on or make changes to this bug.