857980 - [pulsetranslator] Add support for ´locales´ property for release notifications of l10n builds

Status: VERIFIED FIXED

(Webtools :: Pulse, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

In bug 745975 we got a new property for release build notifications. For now it's a temporary workaround until we get bug 857971 implemented.

Pulsetranslator should split such a notification into multiple ones, so that consumers can rely on each individual build.

So the following should trigger 3 notifications with only the locale different:

locales=it,hu,fr

I have asked for an example snippet I can work off without having to wait a week now until the next beta candidate builds are produced.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I have seen some notifications for mobile not desktop yet which look like:

locales: { 'de': 'Success', 'hu': 'Failed', ... }

I will wait until tomorrow when new beta candidate builds will appear.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Henrik Skupin (:whimboo) from comment #1)
> locales: { 'de': 'Success', 'hu': 'Failed', ... }

So it looks like we have different formats between mobile and desktop. :( For todays beta candidate builds we get:

["locales", "fi,fr,fy-NL,ga-IE,gd,gl,gu-IN,he,hi-IN,hr,hu,hy-AM,id,is,it", "SetProperty Step"]

Ben, can we assume that all those repacks were successful or how is a failing repack propagating the failure? We don't want to test broken builds. Thanks.

Comment 3

[[e-mail] (:bhearsum)]

(In reply to Henrik Skupin (:whimboo) from comment #2)
> (In reply to Henrik Skupin (:whimboo) from comment #1)
> > locales: { 'de': 'Success', 'hu': 'Failed', ... }
> 
> So it looks like we have different formats between mobile and desktop. :(
> For todays beta candidate builds we get:
> 
> ["locales", "fi,fr,fy-NL,ga-IE,gd,gl,gu-IN,he,hi-IN,hr,hu,hy-AM,id,is,it",
> "SetProperty Step"]
> 
> Ben, can we assume that all those repacks were successful or how is a
> failing repack propagating the failure? We don't want to test broken builds.
> Thanks.

There's no way to get individual locale status for desktop currently. You can check the overall Build status to see if any locale failed, but you won't know which locale(s) failed.

One workaround for this would be to do a HEAD request on the localized build (eg, https://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/21.0b2-candidates/build1/linux-x86_64/af/firefox-21.0b2.tar.bz2). If that's a 404, that locale failed.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hm, that's not ideal then. :( So our download step will cover that and mark the job as failed. Lets see how this will work.

Jonathan, what is your take here? Shall I add multiple job entries to the queue in such a case? I would say it's better to handle here as in pulsebuildmonitor.

Comment 5

[Jonathan Griffin (:jgriffin)]

We *almost* handle this.  Currently, the translator does HTTP HEAD requests for the build log (for up to 5 minutes) and won't publish a new notification for a build whose log doesn't exist:

https://github.com/mozilla/pulsetranslator/blob/master/pulsetranslator/loghandler.py#L70

For builds, we could also check for the buildurl parameter (if it's present), and depending on whether or not it is present by the time the build log is available, we could add a boolean 'success' flag to the published message.

This way, we'd still be publishing messages for failed builds (which we want, since there are consumers of this data that want to know about failed builds), but the consumer could filter them out easily if it doesn't want them.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Great info about the pulsetranslator internals. Jonathan, that means you are ok that we push multiple notifications (each for a single locale listed) via pulsetranslator?

Comment 7

[Jonathan Griffin (:jgriffin)]

Yes, but if we do that, we should modify pulsebuildmonitor to watch for en-US builds only, by default, but let the caller specify other locales or "all" if they choose.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Jonathan Griffin (:jgriffin) from comment #7)
> Yes, but if we do that, we should modify pulsebuildmonitor to watch for
> en-US builds only, by default, but let the caller specify other locales or
> "all" if they choose.

I don't think we have to do any change in pulsebuildmonitor. With the patch from myself already landed a while back we listen for all locales and can filter out specific ones. See bug 837413 for details.

For pulsebuildmonitor this bug will be totally transparent, except the fact that the same job number is used a couple of times. But I don't think that will be a problem, right?

Comment 9

[Jonathan Griffin (:jgriffin)]

(In reply to Henrik Skupin (:whimboo) from comment #8)
> 
> For pulsebuildmonitor this bug will be totally transparent, except the fact
> that the same job number is used a couple of times. But I don't think that
> will be a problem, right?

I don't think so.  No one is using the job number (except maybe you), so I don't think it matters if it's repeated.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Jonathan, given that we currently do not publish any build notification which does not contain a buildurl, I wonder how important that field is for your tools, and if we can change it to be null. 

A build notification from such a repack job would look like:

{"locale": "ach", "testsurl": null, "previous_buildid": null, "job_number": 3, "build_number": 1, "builddate": 1365432787, "tags": [], "platform": "linux64", "version": "21.0b2", "revision": null, "status": 0, "buildtype": "opt", "product": "firefox", "slave": "bld-linux64-ec2-666", "buildername": "release-mozilla-beta-linux64_repack_1/6", "buildid": "20130408165307", "timestamp": "2013-08-16T13:33:50Z", "key": "build.release-mozilla-beta-linux64_repack_1.3.log_uploaded", "logurl": null, "tree": "release-mozilla-beta", "buildurl": null, "release": "1a25cffd6c65"}

For us it is enough information given that we have the platform, version, product name, and locale of the build. We feed that information into mozdownload and it retrieves the correct build.

Comment 11

[Jonathan Griffin (:jgriffin)]

Setting buildurl to null for cases where we don't have a build url shouldn't cause any problems.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Patch v1

This patch implements the handling of repack notifications. It triggeres individual build notifications for each included locale. As talked with Jonathan on IRC we don't care of the individual build status, but take the overall result. It would be a bit of hacky to build up the build_url given by the sparse properties we get via the pulse notification. At the end I think we in terms of QA are even happy to see it fail on our side to get notified about broken beta and release candidate builds.

Jonathan, one note to the patch. I have used the deepcopy method to ensure to not wrangle with the original builddata variable but really to work on a copy. I hope you also like that. It always reduces the changes to mess up with data.

Comment 13

[Jonathan Griffin (:jgriffin)]

Comment on attachment 791322 [details] [diff] [review]
Patch v1

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

Looks good, thanks.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Landed as:
https://github.com/mozilla/pulsetranslator/commit/7d199faf0ccb8a3cb48530eed2f1a86b9ba65d09

Jonathan, can you push this to production when you have the time? Thanks.

Comment 15

[Jonathan Griffin (:jgriffin)]

deployed

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Jonathan! Lets close it as fixed then. I will verify the behavior once we get new beta builds for Firefox 24.0.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Worked fantastic for 24.0b5! Marking as verified fixed.