Stop relying on push comment, as_slugid from mozilla-taskcluster

RESOLVED FIXED in Firefox -esr60

Status

task
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: dustin, Assigned: dustin)

Tracking

Trunk
mozilla67

Firefox Tracking Flags

(firefox-esr60 fixed, firefox65 fixed, firefox66 fixed, firefox67 fixed)

Details

Attachments

(4 attachments)

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

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

Summary: Stop relying on push comment from mozilla-taskcluster → Stop relying on push comment, as_slugid from mozilla-taskcluster

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.

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.

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

Attachment #9040538 - Attachment description: Bug 1524358 - use taskId, instead of as_slugid('decision'), in .taskcluster.yml → Bug 1524358 - use ownTaskId, instead of as_slugid('decision'), in .taskcluster.yml

Comment 10

3 months ago

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

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

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

Flags: needinfo?(aki)

Comment 14

3 months ago

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

Flags: needinfo?(aki)

Comment 15

3 months ago
Attachment #9040857 - Flags: review?(mozilla)

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?

Updated

3 months ago
Attachment #9040857 - Flags: review?(mozilla)

Comment 17

3 months ago

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

3 months ago
Pushed by dmitchell@mozilla.com:
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 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

Attachment #9040584 - Flags: approval-mozilla-release?
Attachment #9040584 - Flags: approval-mozilla-geckoview65?
Attachment #9040584 - Flags: approval-mozilla-esr60?
Attachment #9040584 - Flags: approval-mozilla-beta?

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

Attachment #9040582 - Flags: approval-mozilla-release?
Attachment #9040582 - Flags: approval-mozilla-geckoview65?
Attachment #9040582 - Flags: approval-mozilla-esr60?
Attachment #9040582 - Flags: approval-mozilla-beta?

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

Attachment #9040538 - Flags: approval-mozilla-release?
Attachment #9040538 - Flags: approval-mozilla-geckoview65?
Attachment #9040538 - Flags: approval-mozilla-esr60?
Attachment #9040538 - Flags: approval-mozilla-beta?

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.

Attachment #9040538 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

[Triage Comment]

Attachment #9040582 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

[Triage Comment]

Attachment #9040584 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Last Resolved: 3 months ago2 months ago
Resolution: --- → FIXED

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

[Triage Comment]
Approved for mozilla-release too.

Attachment #9040538 - Flags: approval-mozilla-release?
Attachment #9040538 - Flags: approval-mozilla-release+
Attachment #9040538 - Flags: approval-mozilla-geckoview65?
Attachment #9040582 - Flags: approval-mozilla-release?
Attachment #9040582 - Flags: approval-mozilla-release+
Attachment #9040582 - Flags: approval-mozilla-geckoview65?
Attachment #9040584 - Flags: approval-mozilla-release?
Attachment #9040584 - Flags: approval-mozilla-release+
Attachment #9040584 - Flags: approval-mozilla-geckoview65?

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.

Attachment #9040538 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Attachment #9040582 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Attachment #9040584 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
You need to log in before you can comment on or make changes to this bug.