Closed Bug 1415391 Opened 2 years ago Closed 2 years ago

followup fixes to bug 1412690 (fennec in-tree relpro)

Categories

(Release Engineering :: Release Automation: Other, enhancement)

enhancement
Not set

Tracking

(firefox58 fixed)

RESOLVED FIXED
Tracking Status
firefox58 --- fixed

People

(Reporter: aki, Assigned: aki)

References

Details

Attachments

(5 files, 1 obsolete file)

Bug 1412690 landed before the change freeze, but before Dustin had a look. This bug is here to track addressing his comments.
Assignee: nobody → aki
Attachment #8926230 - Attachment is obsolete: true
Patch 1 of ? sent to mozreview. The only changes involve adding task.extra.index.rank to fennec release tasks.
I figured I could get this reviewed while we work on the other review comments from bug 1412690.
Comment on attachment 8926239 [details]
bug 1415391 - move release indexes into index_builder('release').

https://reviewboard.mozilla.org/r/197498/#review202922

Nice!
Attachment #8926239 - Flags: review?(dustin) → review+
(Landed / reviewed / written):

_ x x release indexes
_ _ x beetmover-cdns worker-type
_ _ x bbb properties

_ _ _ NEXT_VERSION and BUILD_NUMBER parameters
      https://bugzilla.mozilla.org/show_bug.cgi?id=1412690#c55

_ _ _ product attribute (I'd like to also add shipping-phase or relpro-action or something, so we can filter tasks in the release promotion action target_tasks_methods more elegantly.)
      https://bugzilla.mozilla.org/show_bug.cgi?id=1412690#c59

_ _ _ rework notifications - this is a pretty large set of changes.
      https://bugzilla.mozilla.org/show_bug.cgi?id=1412690#c56
      https://bugzilla.mozilla.org/show_bug.cgi?id=1412690#c58
The shipping-product / shipping-phase patch is in progress. It will change a lot of code while keeping the graph the same. It'll make including/excluding tasks in each action much simpler.

I'm open to work on notifications unless :garbas wants it. We could tackle bug 1412014 (disable notifications on completion) while we're at it, or leave that til later.

(Landed / reviewed / written):

_ x x release indexes
_ _ x beetmover-cdns worker-type
_ _ x bbb properties

_ _ x NEXT_VERSION and BUILD_NUMBER parameters
      https://bugzilla.mozilla.org/show_bug.cgi?id=1412690#c55

_ _ _ product attribute (I'd like to also add shipping-phase or relpro-action or something, so we can filter tasks in the release promotion action target_tasks_methods more elegantly.)
      https://bugzilla.mozilla.org/show_bug.cgi?id=1412690#c59

_ _ _ rework notifications - this is a pretty large set of changes.
      https://bugzilla.mozilla.org/show_bug.cgi?id=1412690#c56
      https://bugzilla.mozilla.org/show_bug.cgi?id=1412690#c58
:garbas is going to work on notifications... we can split those into another bug.

:dustin - do you have time to review? I do know the 2nd patch is needed for Fennec 58.0b3 on Monday. I'll ping the firefox-ci email list.

I do want these changes on maple soon, since they reduce a lot of ugliness. If one or more of these patches are delayed, I can land there sooner for dev work.

(Landed / reviewed / written):

_ x x release indexes
_ _ x beetmover-cdns worker-type
_ _ x bbb properties

_ _ x NEXT_VERSION and BUILD_NUMBER parameters
      https://bugzilla.mozilla.org/show_bug.cgi?id=1412690#c55

_ _ x product attribute (I'd like to also add shipping-phase or relpro-action or something, so we can filter tasks in the release promotion action target_tasks_methods more elegantly.)
      https://bugzilla.mozilla.org/show_bug.cgi?id=1412690#c59
Flags: needinfo?(dustin)
Yeah, I've got about 20 reviews pending but I'll bump this one to the top of the list.
Flags: needinfo?(dustin)
Comment on attachment 8926512 [details]
bug 1415391 - fix beetmover-cdns worker-type schema.

https://reviewboard.mozilla.org/r/197770/#review203748
Attachment #8926512 - Flags: review?(dustin) → review+
Comment on attachment 8926542 [details]
bug 1415391 - move bbb properties under worker:

https://reviewboard.mozilla.org/r/197778/#review203750

::: taskcluster/taskgraph/transforms/task.py:1016
(Diff revision 1)
> +
> +    if worker['properties'].get('tuxedo_server_url'):
> +        resolve_keyed_by(
> +            worker, 'properties.tuxedo_server_url', worker['buildername'],
> +            **config.params
> +        )

If it's useful this could call resolve_keyed_by on all properties, but at the moment this is the one that needs it :)
Attachment #8926542 - Flags: review?(dustin) → review+
Comment on attachment 8926639 [details]
bug 1415391 - parameterize next_version and build_number.

https://reviewboard.mozilla.org/r/197874/#review203752
Attachment #8926639 - Flags: review?(dustin) → review+
Comment on attachment 8927120 [details]
bug 1415391 - add `shipping_phase` and `shipping_product` attributes.

https://reviewboard.mozilla.org/r/198342/#review203754

::: taskcluster/taskgraph/transforms/beetmover_cdns.py:45
(Diff revision 1)
>      Required('worker-type'): optionally_keyed_by('project', basestring),
>      Optional('dependencies'): {basestring: taskref_or_string},
>      Optional('index'): {basestring: basestring},
>      Optional('routes'): [basestring],
> +    Required('shipping-phase'): basestring,
> +    Required('shipping-product'): basestring,

This could refer to `task_description_schema['shipping-xx']` so that it gets the Any(..) validation as early as possible in the transformation process.

::: taskcluster/taskgraph/transforms/push_apk.py:39
(Diff revision 1)
>      Required('worker-type'): basestring,
>      Required('worker'): object,
>      Required('scopes'): None,
>      Required('deadline-after'): basestring,
> +    Required('shipping-phase'): basestring,
> +    Required('shipping-product'): basestring,

same

::: taskcluster/taskgraph/transforms/push_apk_breakpoint.py:36
(Diff revision 1)
>      Required('worker'): object,
>      Required('treeherder'): object,
>      Required('run-on-projects'): list,
>      Required('deadline-after'): basestring,
> +    Required('shipping-product'): basestring,
> +    Required('shipping-phase'): basestring,

same
Attachment #8927120 - Flags: review?(dustin) → review+
(In reply to Dustin J. Mitchell [:dustin] from comment #13)
> Yeah, I've got about 20 reviews pending but I'll bump this one to the top of
> the list.

Ouch. Thank you, and sorry for barging in.

(In reply to Dustin J. Mitchell [:dustin] from comment #17)
> Comment on attachment 8927120 [details]
> bug 1415391 - add `shipping_phase` and `shipping_product` attributes.
> 
> https://reviewboard.mozilla.org/r/198342/#review203754
> 
> ::: taskcluster/taskgraph/transforms/beetmover_cdns.py:45
> (Diff revision 1)
> >      Required('worker-type'): optionally_keyed_by('project', basestring),
> >      Optional('dependencies'): {basestring: taskref_or_string},
> >      Optional('index'): {basestring: basestring},
> >      Optional('routes'): [basestring],
> > +    Required('shipping-phase'): basestring,
> > +    Required('shipping-product'): basestring,
> 
> This could refer to `task_description_schema['shipping-xx']` so that it gets
> the Any(..) validation as early as possible in the transformation process.

I decided not to, so we can't set a shipping-phase or shipping-product of None in this kind. I'll add a comment.
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6108f6cc6a1
move release indexes into index_builder('release'). r=dustin
https://hg.mozilla.org/integration/autoland/rev/136574f7a5e4
fix beetmover-cdns worker-type schema. r=dustin
https://hg.mozilla.org/integration/autoland/rev/0fd32f2a273f
move bbb properties under worker: r=dustin
https://hg.mozilla.org/integration/autoland/rev/598578e14b9f
parameterize next_version and build_number. r=dustin
https://hg.mozilla.org/integration/autoland/rev/7d4154fe9a26
add `shipping_phase` and `shipping_product` attributes. r=dustin
(In reply to Aki Sasaki [:aki] from comment #18)
> > ::: taskcluster/taskgraph/transforms/beetmover_cdns.py:45
> > (Diff revision 1)
> > >      Required('worker-type'): optionally_keyed_by('project', basestring),
> > >      Optional('dependencies'): {basestring: taskref_or_string},
> > >      Optional('index'): {basestring: basestring},
> > >      Optional('routes'): [basestring],
> > > +    Required('shipping-phase'): basestring,
> > > +    Required('shipping-product'): basestring,
> > 
> > This could refer to `task_description_schema['shipping-xx']` so that it gets
> > the Any(..) validation as early as possible in the transformation process.
> 
> I decided not to, so we can't set a shipping-phase or shipping-product of
> None in this kind. I'll add a comment.

I lied. I switched to task_description_schema; if we set these to None, the task just won't show up in the appropriate release promotion action.
Depends on: 1416812
You need to log in before you can comment on or make changes to this bug.