Closed
Bug 1371497
Opened 7 years ago
Closed 7 years ago
Stylo Talos jobs stopped running on m-c
Categories
(Firefox Build System :: Task Configuration, task)
Firefox Build System
Task Configuration
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla55
People
(Reporter: bholley, Assigned: gps)
Details
Attachments
(1 file)
This is a big problem. I suspect bug 1370663 may be related?
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(kmoir)
Assignee | ||
Comment 1•7 years ago
|
||
I'm on it.
Assignee: nobody → gps
Status: NEW → ASSIGNED
Flags: needinfo?(kmoir)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8875963 [details] Bug 1371497 - More robust handling of spaces in variant processing for Talos BBB; https://reviewboard.mozilla.org/r/147366/#review151844 So, basically, we were adding spaces after matching, and attempting to compensate for the spaces by adjusting the formatting string correctly. But then when we started switching on the value of the variant, we forgot to add spaces to the switch cases themselves, and therefore started formatting the job names wrong, and things went downhill from there? ::: taskcluster/taskgraph/transforms/job/mozharness_test.py:450 (Diff revision 2) > if variant in ('stylo', 'stylo-sequential'): > - buildername = '{} {}{} talos {}'.format( > + buildername = '{} {} {} talos {}'.format( > + BUILDER_NAME_PREFIX[platform], > + variant, > + branch, > + test_name > + ) > + elif variant: > + buildername = '{} {} {} talos {}'.format( > BUILDER_NAME_PREFIX[platform], > variant, > branch, > test_name > ) Why are we duplicating these cases? It looks like we don't need separate cases for stylo now...
Assignee | ||
Comment 6•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8875963 [details] Bug 1371497 - More robust handling of spaces in variant processing for Talos BBB; https://reviewboard.mozilla.org/r/147366/#review151844 > Why are we duplicating these cases? It looks like we don't need separate cases for stylo now... Because I refactored this patch while developing and I was too stupid to notice the redundancy in the new version.
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8875963 [details] Bug 1371497 - More robust handling of spaces in variant processing for Talos BBB; https://reviewboard.mozilla.org/r/147366/#review151844 > Because I refactored this patch while developing and I was too stupid to notice the redundancy in the new version. Or not. The intent was correct. I messed up the order things are formatted in.
Comment hidden (mozreview-request) |
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8875963 [details] Bug 1371497 - More robust handling of spaces in variant processing for Talos BBB; https://reviewboard.mozilla.org/r/147366/#review151930 Something between an r=me and an rs=me. Interested in your thoughts on the below. ::: taskcluster/taskgraph/transforms/job/mozharness_test.py:453 (Diff revision 3) > variant, > branch, > test_name > ) > - else: > - buildername = '{} {} {}talos {}'.format( > + elif variant: > + buildername = '{} {} {} talos {}'.format( > BUILDER_NAME_PREFIX[platform], > branch, > variant, That difference is...kind of subtle. :( I wonder, does: ```python if variant in ('stylo', 'stylo_sequential'): name = "{prefix} {variant} {branch} talos {test_name}" elif variant: name = "{prefix} {branch} {variant} talos {test_name}" else: name = "{prefix} {branch} talos {test_name}" buildername = name.format(prefix=BUILDER_NAME_PREFIX[platform], branch=branch, variant=variant, test_name=test_name) ``` read any better to you? Would be nice if we had some explanation of why the stylo ones are switched around in the first place.
Attachment #8875963 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8875963 [details] Bug 1371497 - More robust handling of spaces in variant processing for Talos BBB; https://reviewboard.mozilla.org/r/147366/#review151930 > That difference is...kind of subtle. :( > > I wonder, does: > > ```python > if variant in ('stylo', 'stylo_sequential'): > name = "{prefix} {variant} {branch} talos {test_name}" > elif variant: > name = "{prefix} {branch} {variant} talos {test_name}" > else: > name = "{prefix} {branch} talos {test_name}" > buildername = name.format(prefix=BUILDER_NAME_PREFIX[platform], > branch=branch, > variant=variant, > test_name=test_name) > ``` > > read any better to you? Would be nice if we had some explanation of why the stylo ones are switched around in the first place. Yes, your version reads better. There is a prior inline comment linking to bug 1338871 regarding the ordering. But that bug is resolved. So I dunno what's going on.
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2019434e6aac More robust handling of spaces in variant processing for Talos BBB; r=froydnj
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(gps)
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2019434e6aac
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•6 years ago
|
Product: TaskCluster → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•