1524358 - Stop relying on push comment, as_slugid from mozilla-taskcluster

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

Let's fetch the comment from hg itself when necessary, since hg's pulse messages do not include the comment.

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

The as_slugid(..) context function is also problematic, as hooks has a single, predefined taskId it wants to create.

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

I think I'll solve the second of those by providing a taskId from mozilla-taskcluster that is just set to as_slugid('decision'). I'll add similar support in taskcluster/taskgraph/cron/decision.py. Then I can replace the latter with the former in .taskcluster.yml and everything will keep working for now, and also be future-compatible.

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

https://github.com/taskcluster/mozilla-taskcluster/pull/137

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1524358 - use ownTaskId, instead of as_slugid('decision'), in .taskcluster.yml

This is a forward-looking change to allow evaluation of .taskcluster.yml in
situations where as_slugid is not defined -- such as in the Taskcluster hooks
service.

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

https://github.com/mozilla-releng/scriptworker/pull/309

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=40d64ddc02a46c0416042e864eba55babdd05991

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

Things make a lot more sense if I name it ownTaskId :)

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1524358 - document inputs to .taskcluster.yml more deeply

Comment 9

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1524358 - get commit message locally, rather than from task parameters

Comment 10

[Aki Sasaki (not active)]

If we get a new green try push, I can try to verify that with the scriptworker PR code.

Comment 11

[Dustin J. Mitchell [:dustin] (he/him)]

I'll try again once I've landed the mozilla-taskcluster PR. It wasn't going to work without that :)

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d2fa3726d44c09cf54512ce205d650e1627aafb

Comment 13

[Dustin J. Mitchell [:dustin] (he/him)]

Aki, https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ad87d371673666dda329dbf34ed99d9e1e39c73 looks good (I'll fix the test failure) if you want to verify it.

Comment 14

[Aki Sasaki (not active)]

The decision task passes (verify_cot --task-type decision --cleanup -- IfjKugJaTRyTIvKglqAo8A)

Comment 15

[Aki Sasaki (not active)]

Attachment: puppet - scriptworker 18.1.0

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

Awesome! Once that's landed, are we good to land the m-c patches?

..and once that's done, how should I go about uplifting these?

Comment 17

[Aki Sasaki (not active)]

That's landed, so we can land the m-c patches.
We could uplift with a=release, or go through the standard relman approval process... looks like https://wiki.mozilla.org/Release_Management/Uplift_rules#Guidelines_on_approval_comments

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b7a0772936d3
document inputs to .taskcluster.yml more deeply r=tomprince
https://hg.mozilla.org/integration/autoland/rev/7fb53ad56f85
use ownTaskId, instead of as_slugid('decision'), in .taskcluster.yml r=Callek,tomprince
https://hg.mozilla.org/integration/autoland/rev/5b3dfdf70126
get commit message locally, rather than from task parameters r=tomprince

Comment 19

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 9040584 [details]
Bug 1524358 - get commit message locally, rather than from task parameters

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1524358

User impact if declined

No Firefox builds

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Risk is that scriptworker verification would fail, but Aki has tested that.

String changes made/needed

none

ESR Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for ESR consideration

Needed to build ESR once mozilla-taskcluster is retired

User impact if declined

No Firefox builds

Fix Landed on Version

5b3dfdf70126

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Risk is that scriptworker verification would fail, but Aki has tested that.

String or UUID changes made by this patch

none

GeckoView Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for consideration

Needed to build GeckoView once mozilla-taskcluster is retired

User impact if declined

No GeckoView builds

Fix Landed on Version

5b3dfdf70126

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Risk is that scriptworker verification would fail, but Aki has tested that.

String or UUID changes made by this patch

none

Comment 20

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 9040582 [details]
Bug 1524358 - document inputs to .taskcluster.yml more deeply

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1524358

User impact if declined

No Firefox builds

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Risk is that scriptworker verification would fail, but Aki has tested that.

String changes made/needed

none

ESR Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for ESR consideration

Needed to build ESR once mozilla-taskcluster is retired

User impact if declined

No Firefox builds

Fix Landed on Version

5b3dfdf70126

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Risk is that scriptworker verification would fail, but Aki has tested that.

String or UUID changes made by this patch

none

GeckoView Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for consideration

Needed to build GeckoView once mozilla-taskcluster is retired

User impact if declined

No GeckoView builds

Fix Landed on Version

5b3dfdf70126

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Risk is that scriptworker verification would fail, but Aki has tested that.

String or UUID changes made by this patch

none

Comment 21

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 9040538 [details]
Bug 1524358 - use ownTaskId, instead of as_slugid('decision'), in .taskcluster.yml

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1524358

User impact if declined

No Firefox builds

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Risk is that scriptworker verification would fail, but Aki has tested that.

String changes made/needed

none

ESR Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for ESR consideration

Needed to build ESR once mozilla-taskcluster is retired

User impact if declined

No Firefox builds

Fix Landed on Version

5b3dfdf70126

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Risk is that scriptworker verification would fail, but Aki has tested that.

String or UUID changes made by this patch

none

GeckoView Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for consideration

Needed to build GeckoView once mozilla-taskcluster is retired

User impact if declined

No GeckoView builds

Fix Landed on Version

5b3dfdf70126

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Risk is that scriptworker verification would fail, but Aki has tested that.

String or UUID changes made by this patch

none

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9040538 [details]
Bug 1524358 - use ownTaskId, instead of as_slugid('decision'), in .taskcluster.yml

Changes in build infrastructure, let's uplift to beta since I would like to start beta 5 builds today.

Comment 23

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9040582 [details]
Bug 1524358 - document inputs to .taskcluster.yml more deeply

[Triage Comment]

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9040584 [details]
Bug 1524358 - get commit message locally, rather than from task parameters

[Triage Comment]

Comment 25

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/af516292be1c
https://hg.mozilla.org/releases/mozilla-beta/rev/bbbacf423bdc
https://hg.mozilla.org/releases/mozilla-beta/rev/a142aefab93a

Comment 26

[Dustin J. Mitchell [:dustin] (he/him)]

In case the "No Firefox builds" bit is scary: that would be after we turn off mozilla-taskcluster. That has to get turned off at some point, but not before this is uplifted everywhere it needs to be, or if not uplifted until we find an alternative solution. There's no huge rush to uplift here.

Comment 27

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/b7a0772936d3
https://hg.mozilla.org/mozilla-central/rev/7fb53ad56f85
https://hg.mozilla.org/mozilla-central/rev/5b3dfdf70126

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9040538 [details]
Bug 1524358 - use ownTaskId, instead of as_slugid('decision'), in .taskcluster.yml

[Triage Comment]
Approved for mozilla-release too.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9040538 [details]
Bug 1524358 - use ownTaskId, instead of as_slugid('decision'), in .taskcluster.yml

I'll approve this for esr60 as well, but am deferring on the uplifts to tomprince as part of his regular syncs.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/bbc3115c9077
https://hg.mozilla.org/releases/mozilla-release/rev/ad0d9539cc64
https://hg.mozilla.org/releases/mozilla-release/rev/b04e2261e4c9

Comment 31

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/7afa591a19b4
https://hg.mozilla.org/releases/mozilla-esr60/rev/83a4237d5aa1
https://hg.mozilla.org/releases/mozilla-esr60/rev/1180e2a8f401