469680 - Provide directory link immediately on try submission

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P4)

Description

[Johnathan Nightingale [:johnath]]

There's a very common try server use case that goes:

 - Add a patch to a bug
 - Try server that patch for people to test
 - Once the builds are done, comment on the bug with the directory link so that people can fetch that build

What complicates this process is that while the try server is churning on the patch, the builds directory ( https://build.mozilla.org/tryserver-builds/?C=M;O=D ) shows no sign of it.  This means that the submitter has to poll from time to time, seeing if there's a directory, so that they can comment in the bug and close the loop.  And typically, as soon as (say) the Mac build is done, they'll scoop up the directory, post it in the bug and say something like "Builds still underway, but they should appear here..."  Bugs like bug 430942 are all about this use case.

I suspect that we can know the directory they will land in the second they are submitted.  I propose one of two things:

1) Create the directory immediately.  Yes, it may be empty for a while, and in the case of a failed build, might be permanently empty, but so what?  They're cheap, they're cleaned up in 30 days, and it gives us a live link target right away.

2) Don't change the build process at all, but at least anticipate what the directory WILL be ( timestamp-user@host-buildid ) and output that as a linky on the success page/tinderbox waterfall so that people have a link (albeit not valid yet).

I don't know quite enough about buildbot to make the first thing go, but with pointers to the appropriate source, I could try to put a patch together.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

Ben, creating the dir could be as simple as moving 
 http://mxr.mozilla.org/seamonkey/source/tools/buildbot-configs/tryserver/master.cfg#202
up to 
 http://mxr.mozilla.org/seamonkey/source/tools/buildbot-configs/tryserver/master.cfg#134
? We might get three attempts to create the dir very close together, but that'll probably be OK.

Not sure if we can report it in the submission success page since the timestamp comes from buildbot, and we can't easily pass the tinderbox server a binaryurl (even supposing it would show that before the build was finished).

Comment 2

[Johnathan Nightingale [:johnath]]

(In reply to comment #1)
> Ben, creating the dir could be as simple as moving 
> 
> http://mxr.mozilla.org/seamonkey/source/tools/buildbot-configs/tryserver/master.cfg#202
> up to 
> 
> http://mxr.mozilla.org/seamonkey/source/tools/buildbot-configs/tryserver/master.cfg#134
> ? We might get three attempts to create the dir very close together, but
> that'll probably be OK.

Woot.

> Not sure if we can report it in the submission success page since the timestamp
> comes from buildbot, and we can't easily pass the tinderbox server a binaryurl
> (even supposing it would show that before the build was finished).

So what if we change up the process a little to make it more predictable.  You know better than I whether this is a possibility, but if I can think out loud for a minute... Right now, users can specify a unique ID, otherwise we generate one for them, right?  And then we do:

<timestamp>-<user@host>-<uniqueID>

So what if timestamp became datestamp?  We could know that ahead of time.  For that matter, would it be possible to just dump the timestamp portion of the directory name?  You can still see creation time for the directory in the index listing, still sort by that, but I don't know if it's needed for other reasons.

For that matter, even the uniqueID could be moved up front, if that's a problem to get back to the success page.  AIUI, if I specify an ID it gets used, if not someone (let's call him buildbot?) picks a random string for me. So I can easily write a couple lines of javascript to generate a unique ID by default in the submission page, so that we don't hit the backend one.  And seeing it there might actually prompt more users to replace it with something meaningful, but in either case it could mean that all our directory info was known at submit, and hence could show up on the success page.

Maybe I am talking crazy?

Comment 3

[[e-mail] (:bhearsum)]

> So what if timestamp became datestamp?  We could know that ahead of time.  For
> that matter, would it be possible to just dump the timestamp portion of the
> directory name?  You can still see creation time for the directory in the index
> listing, still sort by that, but I don't know if it's needed for other reasons.
> 

Back in the olden (CVS) times most submitted builds didn't have an ID. Timestamp was included there to help avoid overwriting builds. For Mercurial builds we always have one - either changeset id or whatever the user specified. That's a long way of saying "timestamp in the directory name isn't very useful anymore". Removing it would be fine.

> For that matter, even the uniqueID could be moved up front, if that's a problem
> to get back to the success page.  AIUI, if I specify an ID it gets used, if not
> someone (let's call him buildbot?) picks a random string for me. So I can
> easily write a couple lines of javascript to generate a unique ID by default in
> the submission page, so that we don't hit the backend one.  And seeing it there
> might actually prompt more users to replace it with something meaningful, but
> in either case it could mean that all our directory info was known at submit,
> and hence could show up on the success page.
> 

This sounds like a fine strategy to fine.


You probably already know this, but the code you'll need to look at is here:
http://mxr.mozilla.org/mozilla/source/webtools/buildbot-try/sendchange-ui.pm
and maybe:
http://mxr.mozilla.org/mozilla/source/webtools/buildbot-try/sendchange.cgi

Comment 4

[Johnathan Nightingale [:johnath]]

Attachment: Front end changes

I think this should do, but I need some help figuring out:

 - Where try server actually creates the directories, so that it can be persuaded to use the format that sendchange (now) expects
 - How much we care about the case where identifier is empty - should we put in logic to not link on the success page in that case?
 - Does this break for CVS in ways that are exciting enough to care about?

Comment 5

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

johnath: lots has changed since last comment 5 months ago. 

With all the TryServer-as-a-branch work in bug#429887, there are now emails from TryServer pointing to the builds as soon as builds exist, and better visibility in TBPL. Is that enough to meet the original intent of this bug?

Comment 6

[Johnathan Nightingale [:johnath]]

(In reply to comment #5)
> johnath: lots has changed since last comment 5 months ago. 
> 
> With all the TryServer-as-a-branch work in bug#429887, there are now emails
> from TryServer pointing to the builds as soon as builds exist, and better
> visibility in TBPL. Is that enough to meet the original intent of this bug?

Neat - it's been a while since I've run a tryserver build, so maybe! Help me understand, then - when I filed this bug, I said:

(In reply to comment #0)
> There's a very common try server use case that goes:
> 
>  - Add a patch to a bug
>  - Try server that patch for people to test
>  - Once the builds are done, comment on the bug with the directory link so that
> people can fetch that build
> 
> What complicates this process is that while the try server is churning on the
> patch, the builds directory (
> https://build.mozilla.org/tryserver-builds/?C=M;O=D ) shows no sign of it. 
> This means that the submitter has to poll from time to time, seeing if there's
> a directory, so that they can comment in the bug and close the loop.  

Does your comment suggest that this problem is solved now - people can comment in the bug as soon as they've spun off the tryserver build? Or do they still have to poll (albeit email, instead of tinderbox) to find out what the directory name will be?

Comment 7

[Mike Taylor [:bear]]

The try server will be sending a series of emails for the job(s) - the result will be that the dev will know what the directory will be via email.

Is this enough for now with the idea that as TBPL and Try evolve it will get better?

Comment 8

[Johnathan Nightingale [:johnath]]

Heh - see comment 6. I think this bug is fixed as soon as "people can comment
in the bug as soon as they've spun off the tryserver build."

If that's not realistic or worth doing, you can WONTFIX the bug, but I think what you're describing still makes the process async instead of being a single basically-atomic "push, get directory, comment in bug."

This could also be done as an hg hook, if that's easier - create the directory on push, answer back with "builds will appear at $dir"

Comment 9

[Chris AtLee [:catlee]]

FWIW, this could be done as a custom hook into the hg poller, or a custom scheduler.

Comment 10

[Chris AtLee [:catlee]]

johnath, how's this for a plan:

When we detect the user's push, we email them a message along the lines of

-----------
Thanks for your try submission (http://hg.mozilla.org/try/pushloghtml?changeset=def9f90d44c8).

Builds and logs will be available at http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-def9f90d44c8
-----------

How much of a deal breaker would it be if the directory 404'ed until something showed up there?

Comment 11

[Johnathan Nightingale [:johnath]]

(In reply to comment #10)
> johnath, how's this for a plan:
> 
> When we detect the user's push, we email them a message along the lines of
> 
> -----------
> Thanks for your try submission
> (http://hg.mozilla.org/try/pushloghtml?changeset=def9f90d44c8).
> 
> Builds and logs will be available at
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-def9f90d44c8
> -----------

That would work swimmingly

> How much of a deal breaker would it be if the directory 404'ed until something
> showed up there?

Not very much of one at all, imo. Still lets me mark the bug immediately and go home, if I'm doing a "please build this on all platforms so that I can give it to the reporter to try out a fix" build, which is what prompted this request.

Comment 12

[[e-mail] (:bhearsum)]

(In reply to comment #10)
> johnath, how's this for a plan:
> 
> When we detect the user's push, we email them a message along the lines of
> 
> -----------
> Thanks for your try submission
> (http://hg.mozilla.org/try/pushloghtml?changeset=def9f90d44c8).
> 
> Builds and logs will be available at
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-def9f90d44c8
> -----------
> 
> How much of a deal breaker would it be if the directory 404'ed until something
> showed up there?

It might be good to state explicitly that the directory will be a 404 until a build finishes, to avoid any initial confusion.

Comment 13

[Chris AtLee [:catlee]]

Attachment: Implement ChangeNotifier, and instantiate for try branch.

I also added some configuration variables to test mail notifiers in staging.  See other patch.

Comment 14

[Chris AtLee [:catlee]]

Attachment: Enable mail notifiers to mail real authors in production

Comment 15

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 470499 [details] [diff] [review]
Enable mail notifiers to mail real authors in production

>diff --git a/mozilla/staging_config.py b/mozilla/staging_config.py
> BRANCHES = {
>     'tryserver': {
>         'download_base_url': 'http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox',
>-        'enable_mail_notifier': False,
>+        'enable_mail_notifier': False, # Set to True when testing
>+        'email_override': [], # Set to your address when testing

This style of commenting is preferable to the one used in mozilla-tests/staging_config.py. r+ with them sync'd up.

Comment 16

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 470497 [details] [diff] [review]
Implement ChangeNotifier, and instantiate for try branch.

Ben should look at this too, particularly status/mail.py.

Comment 17

[[e-mail] (:bhearsum)]

Comment on attachment 470497 [details] [diff] [review]
Implement ChangeNotifier, and instantiate for try branch.

Is there potential for the Popen call to block? I think this runs in the main thread, so that'd be pretty sucky if it did. I can't think of why it would, personally.

Comment 18

[Chris AtLee [:catlee]]

Comment on attachment 470497 [details] [diff] [review]
Implement ChangeNotifier, and instantiate for try branch.

changeset:   934:3ec1c2e3203b

Comment 19

[Chris AtLee [:catlee]]

Attachment: Scheduler master needs the ChangeNotifier

Comment 20

[[e-mail] (:bhearsum)]

Comment on attachment 471586 [details] [diff] [review]
Scheduler master needs the ChangeNotifier

Pretty hacky :(

Comment 21

[Chris AtLee [:catlee]]

Comment on attachment 470499 [details] [diff] [review]
Enable mail notifiers to mail real authors in production

This was checked in a while ago as 3ec1c2e3203b

Comment 22

[Chris AtLee [:catlee]]

(In reply to comment #21)
> Comment on attachment 470499 [details] [diff] [review]
> Enable mail notifiers to mail real authors in production
> 
> This was checked in a while ago as 3ec1c2e3203b

Sorry, e17c58e0ec30.

Comment 23

[Chris AtLee [:catlee]]

Comment on attachment 471586 [details] [diff] [review]
Scheduler master needs the ChangeNotifier

changeset:   2988:eb327641141f