Closed Bug 614943 Opened 12 years ago Closed 12 years ago

release e-mail improvements

Categories

(Release Engineering :: General, defect)

All
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bhearsum, Assigned: salbiz)

References

Details

Attachments

(8 files, 6 obsolete files)

2.99 KB, patch
bhearsum
: review+
catlee
: feedback+
bhearsum
: feedback+
bhearsum
: checked-in+
Details | Diff | Splinter Review
3.26 KB, patch
bhearsum
: review+
catlee
: feedback+
bhearsum
: feedback+
bhearsum
: checked-in+
Details | Diff | Splinter Review
1.77 KB, patch
catlee
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
2.44 KB, patch
catlee
: review+
bhearsum
: review+
dustin
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
3.08 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
3.97 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
2.53 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
4.57 KB, patch
bhearsum
: review+
rail
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
Using this bug to track various improvements to the automatic release mail.

To start with, staging mail should be easily distinguishable from production mail, probably by the subject line.
I was thinking if we could put all emails wrt a release into the same thread? (Instead of sending each in a new thread)

I know in rel-drivers we have not been doing that way (we send one separate email for each releng action) but we might want to keep in one thread since it is now obvious when a releng (person) is replying to a thread and when the release automation is emailing (release@).

Do we have a list of builders that we DON'T want to email the state to rel-drivers? or do we want to send all?

Another improvement I thought is if we could send only one email when _all_ l10n builders are done instead of one per L10n builder.
Tested in staging, with a sendchange and a bunch of dummy steps.
Subject lines end up like: [staging-release] Tagging started for Firefox 3.6.11 build3

Unfortunately, this will require a reconfig, as interpolating the staging variable into the message templates doesn't really satisfy this requirement
Attachment #493415 - Flags: feedback?(catlee)
Attachment #493415 - Flags: feedback?(bhearsum)
(In reply to comment #1)
> Do we have a list of builders that we DON'T want to email the state to
> rel-drivers? or do we want to send all?
> 

Is there anything specific you want not showing up? We have two pools of recipients for notification, the general group which gets everything(release@), and a special group (which for now, is also just release@)
which gets a bunch of specific steps, and only if they pass, (This is the one that will probably be switched over to r-d in the near future).
I don't think they are interested on the "source" builder and/or partner repacks. I am not sure how interested they will be about the verification ones. We might want to ask them once we are closer to enable it on production.
Comment on attachment 493415 [details] [diff] [review]
add a tag to subject line to differentiate staging

Are you going to adjust the templates to get rid of the newline that appears at the beginning of the notification messages as well?
Done, since this patch only touches the templates, it can be landed safely without a reconfig.
Attachment #493420 - Flags: feedback?(catlee)
Attachment #493420 - Flags: feedback?(bhearsum)
Attachment #493420 - Flags: feedback?(bhearsum) → feedback+
Comment on attachment 493415 [details] [diff] [review]
add a tag to subject line to differentiate staging

Can you name the variable something other than "tag"? In the context of releases, it's confusing.
Done.
Attachment #493415 - Attachment is obsolete: true
Attachment #493422 - Flags: feedback?(catlee)
Attachment #493422 - Flags: feedback?(bhearsum)
Attachment #493415 - Flags: feedback?(catlee)
Attachment #493415 - Flags: feedback?(bhearsum)
Attachment #493422 - Flags: feedback?(bhearsum) → feedback+
Rail pointed out in IRC that the current method of determining the platform for messages was broken. I have attached a fix here.
Attachment #493820 - Flags: feedback?(rail)
Attachment #493820 - Flags: feedback?(catlee)
Comment on attachment 493820 [details] [diff] [review]
fix the platform check in the message code

looks good
Attachment #493820 - Flags: feedback?(rail) → feedback+
Attachment #493420 - Flags: feedback?(catlee) → feedback+
Attachment #493422 - Flags: feedback?(catlee) → feedback+
Comment on attachment 493820 [details] [diff] [review]
fix the platform check in the message code

Please add a comment explaining what this is doing...it took me several minutes to understand what you're trying to do!
Attachment #493820 - Flags: feedback?(catlee) → feedback+
Done. Tests out in staging.
Attachment #493820 - Attachment is obsolete: true
Attachment #494703 - Flags: review?(catlee)
Attachment #494703 - Flags: review?(catlee) → review+
Flags: needs-reconfig?
Planning to do a reconfig with this tomorrow morning.
Flags: needs-reconfig? → needs-reconfig+
Comment on attachment 493420 [details] [diff] [review]
delete leading newline

changeset:   1236:3a8a344f406f
Attachment #493420 - Flags: review+
Attachment #493420 - Flags: checked-in+
Attachment #493422 - Flags: review+
Attachment #493422 - Flags: checked-in+
Attachment #494703 - Flags: checked-in+
Masters have been updated.
Flags: needs-reconfig+
Uses a Dummy Builder fired off a dependent on the repack scheduler. In this patch, that email is only sent to the PassRecipients  bucket (in future r-d@), with all chunk status emails still heading through to release@. Tested on staging. We may want to craft a custom message template for this step.
Attachment #496330 - Flags: review?(catlee)
Attachment #496330 - Flags: review?(bhearsum)
Comment on attachment 496330 [details] [diff] [review]
Send an 'all repacks complete' email when all l10n chunks finish.

Do we want this per-platform, or across all platforms?
Comment on attachment 496330 [details] [diff] [review]
Send an 'all repacks complete' email when all l10n chunks finish.

This seems OK to me, and I don't know of any other way to accurately fire a message when a set of repacks finish. Dustin, I'd be curious to know if you have any thoughts here.
Attachment #496330 - Flags: review?(dustin)
Attachment #496330 - Flags: review?(bhearsum)
Attachment #496330 - Flags: review+
Comment on attachment 496330 [details] [diff] [review]
Send an 'all repacks complete' email when all l10n chunks finish.

Nope, this is the best we've got until we implement Flocks.  Well, the other option is to send the mail in a buildstep, but I think Syed's solution is better.
Attachment #496330 - Flags: review?(dustin) → review+
Attachment #496330 - Flags: review?(catlee) → review+
Comment on attachment 496330 [details] [diff] [review]
Send an 'all repacks complete' email when all l10n chunks finish.

Landed on default: changeset:   1318:6d858c31b9bb
Attachment #496330 - Flags: checked-in+
tested these on my latest staging run
Attachment #499334 - Flags: review?(bhearsum)
individual chunks now go: %release: completed repack_x/y on %platform
all-finished now go: %release: completed l10n repacks on %platform
updates now go: %release: updates available on betatest
Attachment #499336 - Flags: review?(bhearsum)
Attachment #499334 - Flags: review?(bhearsum) → review+
Comment on attachment 499336 [details] [diff] [review]
add platform to repack emails, add updates template

Per IRC, we're going to symlink most of these to master copies.
Attachment #499336 - Attachment is obsolete: true
Attachment #499336 - Flags: review?(bhearsum)
Attached patch link templates (obsolete) — Splinter Review
Done.
Attachment #499357 - Flags: review?(bhearsum)
Comment on attachment 499357 [details] [diff] [review]
link templates

I think you typoed "macosx", looks fine otherwise.
Attachment #499357 - Flags: review?(bhearsum) → review-
Attached patch fix typosSplinter Review
d'oh, old templates stuck around after I rsync-ed the templates dir in staging.
Attachment #499357 - Attachment is obsolete: true
Attachment #499378 - Flags: review?(bhearsum)
Comment on attachment 499378 [details] [diff] [review]
fix typos

Landed on default: changeset:   1321:3c103d673c4f
Attachment #499378 - Flags: review?(bhearsum)
Attachment #499378 - Flags: review+
Attachment #499378 - Flags: checked-in+
Comment on attachment 499334 [details] [diff] [review]
drop unnecessary mail to r-d

I landed this yesterday.
Attachment #499334 - Flags: checked-in+
From e-mail: we need to send links to the builds as they become available.
Attached patch include ftpURL in emails (obsolete) — Splinter Review
generate a generic ftp URL for the release and add platform specific information as available.
Attachment #500112 - Flags: review?(bhearsum)
Attached patch include ftpURL in templates (obsolete) — Splinter Review
add ftpURL to builds, repacks and signing templates
Attachment #500113 - Flags: review?(bhearsum)
Comment on attachment 500112 [details] [diff] [review]
include ftpURL in emails

>+    def genericFtpUrl():
>+        """ Generate an FTP URL pointing to the uploaded release builds for
>+        sticking into release notification messages """
>+        return makeCandidatesDir(
>+            releaseConfig['productName'],
>+            releaseConfig['version'],
>+            releaseConfig['buildNumber'],
>+            protocol='http',
>+            server=branchConfig['stage_server'])

IIRC, protocol should be 'ftp', because Apache has some rewrite rules, which prevent accessing the candidate directory directly. Not sure if they work for platform specific directories.
Comment on attachment 500113 [details] [diff] [review]
include ftpURL in templates

>diff --git a/mozilla/release_templates/win32_build_success b/mozilla/release_templates/win32_build_success
>--- a/mozilla/release_templates/win32_build_success
>+++ b/mozilla/release_templates/win32_build_success
>@@ -1,4 +1,6 @@
>-%(releaseName)s: %(platform)s builds available
>-%(platform)s unsigned builds available for %(releaseName)s
>+%(releaseName)s: %(platform)s en-US build available

The subject here needs to say "unsigned" as well.

r=me with that changed.
Comment on attachment 500112 [details] [diff] [review]
include ftpURL in emails

What does the FTP URL end up looking like post-signing? The root of the candidates directory?

As Rail suggested, we should use ftp:// as the protocol, to make sure the URLs are always accessible.
Attachment #500112 - Flags: review?(bhearsum) → review-
Fixed subject line of the win32 build template
Attachment #500113 - Attachment is obsolete: true
Attachment #500190 - Flags: review?(bhearsum)
Attachment #500113 - Flags: review?(bhearsum)
Attached patch use ftp:// linksSplinter Review
Done, using ftp:// URLs
Attachment #500192 - Flags: review?(rail)
Attachment #500192 - Flags: review?(bhearsum)
Attachment #500190 - Flags: review?(bhearsum) → review+
Attachment #500192 - Flags: review?(bhearsum) → review+
Attachment #500192 - Flags: review?(rail) → review+
Attachment #500112 - Attachment is obsolete: true
Comment on attachment 500190 [details] [diff] [review]
include ftpURL in templates-v2

changeset:   3716:efa343702b04
Attachment #500190 - Flags: checked-in+
Comment on attachment 500192 [details] [diff] [review]
use ftp:// links

changeset:   1334:ad2c3b1788f6
Attachment #500192 - Flags: checked-in+
The latest patches need to be merged to production still, but after that I believe we're all done here.
These are all landed in production now.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.