709891 - Add support to download a daily build as given by a build id

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Automation, defect)

Description

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

We have to add support to download a build as given by the build id. For example a Nightly build with the id '20111212031122' would have to be downloaded from:

/pub/mozilla.org/firefox/nightly/2011/12/2011-12-12-03-11-22-mozilla-central/

That means for determining the date and # of build for that day we have to split up the build id into its datetime components.

Comment 1

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

Attachment: Patch v1

This patch adds the feature to download a build by the given build id. It also prepares for the build-X feature we want to have for updates. I had to do a lot of refactoring to make that possible. Now we always rely on the date for downloading a given build. We can use this to operate with days.

Further updates:
* If the index has not been given we do not longer default to the first found build but the latest one. That makes it safer and probably is what the user wants in any way
* Some command line option updates
* Safer concatenation of strings

Comment 2

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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

This looks great!

Whilst testing I found an issue with downloading where a build folder exists but the specified platform is missing. This seems like something we should catch. You may want to open this as a separate bug though. The following is what I got:

./download.py --type=daily --branch=elm --platform=mac --date=2011-12-09
Retrieving list of builds from http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/2011/12/
Downloading build: http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/2011/12/2011-12-09-18-54-51-elm/None

I also have a minor nit regarding the safer concatenation of strings. It appears this has been done in some places but not others, see lines 63, 119, 218, 278, and 417. In some of these instances, could we even use '/'.join([foo, bar])

Comment 3

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

Attachment: Patch v2

(In reply to Dave Hunt (:davehunt) from comment #2)
> ./download.py --type=daily --branch=elm --platform=mac --date=2011-12-09
> Retrieving list of builds from
> http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/2011/12/
> Downloading build:
> http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/2011/12/2011-12-09-
> 18-54-51-elm/None

Good catch. I haven't really tested all kinds of other developer branches. I think there are still some other issues we should address later on. But in this mentioned case we have to raise an exception. Fixed that.

> I also have a minor nit regarding the safer concatenation of strings. It
> appears this has been done in some places but not others, see lines 63, 119,
> 218, 278, and 417. In some of these instances, could we even use
> '/'.join([foo, bar])

Agreed. I will use join, which is also the proposed method in the Python docs. But I don't think its useful when we have only one operand, as in line 218.

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 581621 [details] [diff] [review]
Patch v2

Looks good to me. Do we have anything running on a cron job (or similar) that relies on the --build command line option?

Comment 5

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

Well spotted Dave. We have a dependency to the testrun_release.py script. Given the change of the CLI option, it will also have to be updated. Otherwise we don't have anything else relying on the download script. I will come up with the final patch soonish.

Comment 6

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

Attachment: Patch v3

Updated patch which fixes the testrun_release.py script and also changes the index to build_number.

Comment 7

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 581713 [details] [diff] [review]
Patch v3

Looks great!

Comment 8

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

Attachment: Patch v4

Ouch, forgot to update download.py. Thankfully run --help and have seen that it was still using --index.

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 581800 [details] [diff] [review]
Patch v4

I should have caught this in the review. Looks good now.

Comment 10

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

Landed as:
http://hg.mozilla.org/qa/mozmill-automation/rev/5f0ebd00cf21