1343718 - Miscellaneous tidying of artifacts download code

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1343718 - Avoid requiring an objdir when simply importing mozbuild.artifacts.

Review commit: https://reviewboard.mozilla.org/r/129344/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129344/

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1343718 - Don't display download progress when we don't know the download size in advance.

In most cases, the HTTP response for the download will contain the
content-length, but if some error happens (e.g. authentication error),
there might not be one, and the download fails with a TypeError for a
division by None, instead of failing with a more friendly error message
about the HTTP error.

Review commit: https://reviewboard.mozilla.org/r/129346/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129346/

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1343718 - Limit artifact downloads by size rather than by number.

Review commit: https://reviewboard.mozilla.org/r/129348/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129348/

Comment 4

[Mike Hommey [:glandium]]

I now realize that the last patch doesn't really do what I wanted it to do, but all things considered, it's not entirely bad, so I'll just leave it that way, unless told otherwise. (it makes the size and file limits apply to the files that were downloaded previously, which means in practice the size limit ends up being the hard limit (1GB) + the size of all the downloads from the current session)

Comment 5

[Mike Hommey [:glandium]]

I also now realize that I broke some of the mach artifact subcommands, namely: clear-cache, last and print-cache. The latter two would need the pickle data to be kept... OTOH the "last" command was already broken: bug 1341205.

Is it worth keeping those two commands working? (I can see an interest in clear-cache)

Comment 6

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8857361 [details]
Bug 1343718 - Avoid requiring an objdir when simply importing mozbuild.artifacts.

https://reviewboard.mozilla.org/r/129344/#review132272

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8857362 [details]
Bug 1343718 - Don't display download progress when we don't know the download size in advance.

https://reviewboard.mozilla.org/r/129346/#review132274

Comment 8

[Mike Hommey [:glandium]]

For the record, I'm about to attach a new version of the third patch, which is going to, in fact, address comment 4, even though I said I wouldn't do that.

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8857361 [details]
Bug 1343718 - Avoid requiring an objdir when simply importing mozbuild.artifacts.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129344/diff/1-2/

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8857362 [details]
Bug 1343718 - Don't display download progress when we don't know the download size in advance.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129346/diff/1-2/

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8857363 [details]
Bug 1343718 - Limit artifact downloads by size rather than by number.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129348/diff/1-2/

Comment 12

[Mike Hommey [:glandium]]

Filed bug 1356101 for the more general issue of the lack of sync between build/virtualenv_packages.txt and build/mach_bootstrap.py

Comment 13

[Mike Hommey [:glandium]]

Err, the unit test fails randomly on automation.

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8857361 [details]
Bug 1343718 - Avoid requiring an objdir when simply importing mozbuild.artifacts.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129344/diff/2-3/

Comment 15

[Mike Hommey [:glandium]]

Comment on attachment 8857362 [details]
Bug 1343718 - Don't display download progress when we don't know the download size in advance.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129346/diff/2-3/

Comment 16

[Mike Hommey [:glandium]]

Comment on attachment 8857363 [details]
Bug 1343718 - Limit artifact downloads by size rather than by number.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129348/diff/2-3/

Comment 17

[Mike Hommey [:glandium]]

Comment on attachment 8857361 [details]
Bug 1343718 - Avoid requiring an objdir when simply importing mozbuild.artifacts.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129344/diff/3-4/

Comment 18

[Mike Hommey [:glandium]]

Comment on attachment 8857362 [details]
Bug 1343718 - Don't display download progress when we don't know the download size in advance.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129346/diff/3-4/

Comment 19

[Mike Hommey [:glandium]]

Comment on attachment 8857363 [details]
Bug 1343718 - Limit artifact downloads by size rather than by number.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129348/diff/3-4/

Comment 20

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8857363 [details]
Bug 1343718 - Limit artifact downloads by size rather than by number.

https://reviewboard.mozilla.org/r/129348/#review132668

Comment 21

[Mike Hommey [:glandium]]

Comment on attachment 8857361 [details]
Bug 1343718 - Avoid requiring an objdir when simply importing mozbuild.artifacts.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129344/diff/4-5/

Comment 22

[Mike Hommey [:glandium]]

Comment on attachment 8857362 [details]
Bug 1343718 - Don't display download progress when we don't know the download size in advance.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129346/diff/4-5/

Comment 23

[Mike Hommey [:glandium]]

Comment on attachment 8857363 [details]
Bug 1343718 - Limit artifact downloads by size rather than by number.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129348/diff/4-5/

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56cb728c5f74
Avoid requiring an objdir when simply importing mozbuild.artifacts. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/b5a3e4cb5090
Don't display download progress when we don't know the download size in advance. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/3717bedc62f9
Limit artifact downloads by size rather than by number. r=chmanchester

Comment 25

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/56cb728c5f74
https://hg.mozilla.org/mozilla-central/rev/b5a3e4cb5090
https://hg.mozilla.org/mozilla-central/rev/3717bedc62f9