Closed Bug 1212942 Opened 4 years ago Closed 4 years ago

All builds should have date-based routes

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: julienw, Assigned: mshal)

References

Details

(Keywords: qablocker)

Attachments

(1 file, 1 obsolete file)

Some builds have nice date-based routes (eg: [1]) but not all of them have these. For example [2] does not have such a route. It would be nice if all builds would have this automatically.

[1] https://tools.taskcluster.net/index/artifacts/#gecko.v2.mozilla-central.nightly/gecko.v2.mozilla-central.nightly
[2] https://tools.taskcluster.net/task-inspector/#SW1DUA5eTCm3OSyDhcWyjQ/
Appears that the post-build tasks just need to make use of this method [1] similar to how builds and tests use it.


[1] https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/mach_commands.py#110
I'm not sure what you mean by #c1 - can you clarify? This patch seems to work. Here's an example in garbage.staging.mshal-testing:

https://tools.taskcluster.net/index/#garbage.staging.mshal-testing.gecko.v2.try.pushdate.2015.10.12.20151012210703/garbage.staging.mshal-testing.gecko.v2.try.pushdate.2015.10.12.20151012210703

Just review the testing/taskcluster bits for now. If those are fine I'll get a mozharness person to review the mozharness changes.
Assignee: nobody → mshal
Attachment #8673804 - Flags: review?(garndt)
Depends on: 1214758
This depends on bug 1214758, since adding the pushdate routes pushes us over the 10-route limit for st-an builds (which have bogus extra routes).
Comment on attachment 8673804 [details] [diff] [review]
0001-Bug-1212942-Add-pushdate-routes.patch

Review of attachment 8673804 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, just a couple of questions.  Thanks!

::: testing/taskcluster/mach_commands.py
@@ +204,5 @@
>                  caches.pop(cache)
>      except KeyError:
>          pass
>  
> +def query_pushinfo(repository, revision):

Ah darn, too bad we couldn't easily just pull this from hgtool.

@@ +363,5 @@
>  
>          cmdline_interactive = params.get('interactive', False)
>  
> +        # Default to current time if querying the head rev fails
> +        pushdate = time.strftime('%Y%m%d%H%M%S')

Do we want to pass in time.gmtime() to strftime?

::: testing/taskcluster/routes.json
@@ +5,5 @@
>          "{index}.gecko.v2.{project}.latest.{build_product}.{build_name}-{build_type}"
>      ],
>      "nightly": [
>          "{index}.gecko.v2.{project}.nightly.{year}.{month}.{day}.revision.{head_rev}.{build_product}.{build_name}-{build_type}",
> +        "{index}.gecko.v2.{project}.nightly.{year}.{month}.{day}.pushdate.{pushdate}.{build_product}.{build_name}-{build_type}",

Curious why nightly gets these two almost similar routes?  Seems just the one key "pushdate" is moved.
Attachment #8673804 - Flags: review?(garndt) → review+
(In reply to Greg Arndt [:garndt] from comment #4)
> ::: testing/taskcluster/mach_commands.py
> @@ +204,5 @@
> >                  caches.pop(cache)
> >      except KeyError:
> >          pass
> >  
> > +def query_pushinfo(repository, revision):
> 
> Ah darn, too bad we couldn't easily just pull this from hgtool.

Oh, is there an easier way to do it? I'd prefer not to have to add this code if the info is available elsewhere :)

> 
> @@ +363,5 @@
> >  
> >          cmdline_interactive = params.get('interactive', False)
> >  
> > +        # Default to current time if querying the head rev fails
> > +        pushdate = time.strftime('%Y%m%d%H%M%S')
> 
> Do we want to pass in time.gmtime() to strftime?

Good idea. Added.

> 
> ::: testing/taskcluster/routes.json
> @@ +5,5 @@
> >          "{index}.gecko.v2.{project}.latest.{build_product}.{build_name}-{build_type}"
> >      ],
> >      "nightly": [
> >          "{index}.gecko.v2.{project}.nightly.{year}.{month}.{day}.revision.{head_rev}.{build_product}.{build_name}-{build_type}",
> > +        "{index}.gecko.v2.{project}.nightly.{year}.{month}.{day}.pushdate.{pushdate}.{build_product}.{build_name}-{build_type}",
> 
> Curious why nightly gets these two almost similar routes?  Seems just the
> one key "pushdate" is moved.

Yeah, on second thought I don't think it makes sense to add these to the nightly routes. We already have {year}.{month}.{day} there, which really serves the same purpose. I've removed the pushdate ones from here.
See Also: → 1201771
As mentioned in this comment https://bugzilla.mozilla.org/show_bug.cgi?id=1201771#c10 this will be a major qa blocker if PVT goes down before this is implemented.
Keywords: qablocker
Asking :garndt for re-review of the Taskcluster changes (including gmtime, removing nightly routes).

Also asking :catlee for feedback on the routes. They currently look like this:

gecko.v2.try.pushdate.2015.10.20.20151020175727.firefox.{linux-opt, etc}

Does that seem reasonable? Or should they be grouped into {year}.{month} instead of {year}.{month}.{day}?
Attachment #8673804 - Attachment is obsolete: true
Attachment #8676444 - Flags: review?(garndt)
Attachment #8676444 - Flags: feedback?(catlee)
Comment on attachment 8676444 [details] [diff] [review]
0001-Bug-1212942-Add-pushdate-routes.patch

r?jlund for the mozharness changes.
Attachment #8676444 - Flags: review?(jlund)
Attachment #8676444 - Flags: feedback?(catlee) → feedback+
Attachment #8676444 - Flags: review?(garndt) → review+
Comment on attachment 8676444 [details] [diff] [review]
0001-Bug-1212942-Add-pushdate-routes.patch

Review of attachment 8676444 [details] [diff] [review]:
-----------------------------------------------------------------

mh parts look good :)

be nice to take this code duplication between the three build scripts and do something like fmt.update(generate_common_template_kwargs()) but I see why they are independent while things are in flux
Attachment #8676444 - Flags: review?(jlund) → review+
https://hg.mozilla.org/mozilla-central/rev/e1bd4c97811f
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
No longer blocks: 1227780
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.