Closed Bug 1415391 Opened 4 years ago Closed 4 years ago

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

Categories

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

enhancement
Not set
normal

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.
You need to log in before you can comment on or make changes to this bug.