1343349 - [tcmigration] add gecko.v2 routes for nightly BM-S tasks

Status: RESOLVED WONTFIX

(Release Engineering :: Release Automation, defect)

Comment 1

[Mihai Tabara [:mtabara]⌚️GMT]

@rail: 
a) can you please tell me again why are we doing this? 
b) @Callek suggested we should also rope :mshal into the conversation to figure out the namespace.
c) do we need this for beetmover checksums as well?
d) the following patch adds up something like this:

"index.gecko.v2.mozilla-central.nightly.revision.71224049c0b52ab190564d3ea0eab089a159a4cf.Firefox.beetmover-tr-signing-l10n-linux64-nightly-3/opt"
Not sure if this is what you had in mind.

Comment 2

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: Bug 1343349 - add gecko.v2 routes for nightly BM-S. DONTBUILD

Review commit: https://reviewboard.mozilla.org/r/116094/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116094/

Comment 3

[Rail Aliiev [:rail]]

Comment on attachment 8842201 [details]
Bug 1343349 - add gecko.v2 routes for nightly BM-S.  DONTBUILD

https://reviewboard.mozilla.org/r/116094/#review117802

I'm not that familiar with this code. It'd be great to get a review from someone familiar. Also, mshal is a good peer for the naming schema.

::: taskcluster/taskgraph/transforms/beetmover.py:230
(Diff revision 1)
> +def add_beetmover_routes(config, jobs):
> +    """Add specific gecko.v2.routes"""
> +    for job in jobs:
> +        platform = job['attributes']['build_platform']
> +        if platform in ('android-api-15', 'android-x86'):
> +            product = 'Fennec'

AFAIK we use lover case products in routes.

::: taskcluster/taskgraph/transforms/beetmover.py:232
(Diff revision 1)
> +    for job in jobs:
> +        platform = job['attributes']['build_platform']
> +        if platform in ('android-api-15', 'android-x86'):
> +            product = 'Fennec'
> +        elif platform in ('linux64-nightly', 'linux-nightly'):
> +            product = 'Firefox'

The same here.

Comment 4

[Rail Aliiev [:rail]]

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #1)
> @rail: 
> a) can you please tell me again why are we doing this? 

We need these routes to be able to resolve these tasks in release running using the index service.

> b) @Callek suggested we should also rope :mshal into the conversation to
> figure out the namespace.

Agree

> c) do we need this for beetmover checksums as well?

Are those the files with "checksums.asc" files? If yes then yes.

> d) the following patch adds up something like this:
> 
> "index.gecko.v2.mozilla-central.nightly.revision.
> 71224049c0b52ab190564d3ea0eab089a159a4cf.Firefox.beetmover-tr-signing-l10n-
> linux64-nightly-3/opt"
> Not sure if this is what you had in mind.

If would be sufficient for release runner.

Comment 5

[Rail Aliiev [:rail]]

BTW, can you land the final patch to jamun first, so we can verify it.

Comment 6

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 8842201 [details]
Bug 1343349 - add gecko.v2 routes for nightly BM-S.  DONTBUILD

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116094/diff/1-2/

Comment 7

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Rail Aliiev [:rail] ⌚️ET from comment #5)
> BTW, can you land the final patch to jamun first, so we can verify it.

Of course! Once it's r+'ed I'll land it there.

(In reply to Rail Aliiev [:rail] ⌚️ET from comment #4)
> Are those the files with "checksums.asc" files? If yes then yes.

Yes. I've refactored the code in a better way and I've included the checks for beetmover-checksums as well.

@Callek:
Sorry for stealing you from mac signing stuff for a bit but I was wondering if you can r? this patch. You have a far better context than I do when it comes to this.

@mshal:
Hi! We need these routes to be able to resolve these tasks in release running using the index service. Does this namespace convention fit well from your standpoint?

"index.gecko.v2.mozilla-central.nightly.revision.71224049c0b52ab190564d3ea0eab089a159a4cf.Firefox.beetmover-tr-signing-l10n-linux64-nightly-3/opt"

Comment 8

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #6)
> Comment on attachment 8842201 [details]
> Bug 1343349 - add gecko.v2 routes for nightly BM-S.
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/116094/diff/1-2/

@Callek, please hold on from reviewing or if done that already, please s,r?,f?. 
I want to follow-up with some improvements changes ;)

Comment 9

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 8842201 [details]
Bug 1343349 - add gecko.v2 routes for nightly BM-S.  DONTBUILD

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116094/diff/2-3/

Comment 10

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #8)
> (In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #6)
> > Comment on attachment 8842201 [details]
> > Bug 1343349 - add gecko.v2 routes for nightly BM-S.
> > 
> > Review request updated; see interdiff:
> > https://reviewboard.mozilla.org/r/116094/diff/1-2/
> 
> @Callek, please hold on from reviewing or if done that already, please
> s,r?,f?. 
> I want to follow-up with some improvements changes ;)

Meh, I thought I could do something smarter with adding those routes somehow in transforms:task but failed in the attempt. :/ At least I moved the util code to util/scriptworker.py to reuse that file instead of creating a new one.

Comment 11

[Justin Wood (:Callek)]

Comment on attachment 8842201 [details]
Bug 1343349 - add gecko.v2 routes for nightly BM-S.  DONTBUILD

https://reviewboard.mozilla.org/r/116094/#review117848

Clearing review request until :mshal comments about the route design (since that may influence more changes)

::: taskcluster/taskgraph/util/scriptworker.py:192
(Diff revision 3)
>      BALROG_SERVER_SCOPES
>  )
> +
> +
> +def get_beetmover_routes(config, job):
> +    """Bug 1343349 - temporary hack to add enrich beetmover tasks with gecko

Nit: "add enrich"

Only one of those words please.

Also please link to a Bug that indicates the "followup" item to do here (since you call this a temporary hack, want to be sure there is a bug that indicates what the non-temp solution is)

::: taskcluster/taskgraph/util/scriptworker.py:204
(Diff revision 3)
> +        product = 'fennec'
> +    elif platform in ('linux64-nightly', 'linux-nightly',
> +                      'linux64-nightly-l10n', 'linux-nightly-l10n'):
> +        product = 'firefox'
> +    else:
> +        return []

Is this `else` a valid branch, or should we raise `Exception("Unknown platform not supported")` here?

Comment 12

[Mihai Tabara [:mtabara]⌚️GMT]

Note to self: do what Callek suggested via IRC.

Callek> mtabara: I bet mshal would find Bug 1343349 better if you did a diff of the route changes with a form of my commands from https://bugzilla.mozilla.org/show_bug.cgi?id=1334624#c0 (specifically "before patch" vs "after patch")

Comment 13

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: routes_difference.diff

Provided by: (Callek++ for suggesting this)

$ hg out -r . central
$ ./mach taskgraph full --json --parameters ~/Downloads/parameters_central_linux_nightly.yml > ~/Downloads/jobs_test1.json
$ cat ~/Downloads/jobs_test1.json | jq '[. | to_entries[] | select(.key) | .value = .value.task.routes ] | from_entries' > routes_before.json
$ hg import (mypatch)
$ ./mach taskgraph full --json --parameters ~/Downloads/parameters_central_linux_nightly.yml > ~/Downloads/jobs_test1.json
$ cat ~/Downloads/jobs_test1.json | jq '[. | to_entries[] | select(.key) | .value = .value.task.routes ] | from_entries' > routes_after.json
$ diff -U10 routes_after.json routes_before.json > routes_difference.diff

Comment 14

[Mihai Tabara [:mtabara]⌚️GMT]

Killing this because of (not only) https://reviewboard.mozilla.org/r/116670/diff/1#index_header from bug 1343935 + some others tasks to handle this instead.