Closed Bug 1343718 Opened 7 years ago Closed 7 years ago

Miscellaneous tidying of artifacts download code

Tracking

(firefox55 fixed)

Status:

RESOLVED FIXED

Milestone:

mozilla55

Tracking Flags:

Tracking

Status

firefox55

---

fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(3 files)

Bug 1343718 - Avoid requiring an objdir when simply importing mozbuild.artifacts. 7 years ago Mike Hommey [:glandium] 59 bytes, text/x-review-board-request	chmanchester : review+	Details
Bug 1343718 - Don't display download progress when we don't know the download size in advance. 7 years ago Mike Hommey [:glandium] 59 bytes, text/x-review-board-request	chmanchester : review+	Details
Bug 1343718 - Limit artifact downloads by size rather than by number. 7 years ago Mike Hommey [:glandium] 59 bytes, text/x-review-board-request	chmanchester : review+	Details

Mike Hommey [:glandium]

Assignee

Description

•

7 years ago

      No description provided.

Mike Hommey [:glandium]

Assignee

Updated

•

7 years ago

Depends on: 1341215

Mike Hommey [:glandium]

Assignee

Updated

•

7 years ago

Summary: Miscellaneous tidying of artifacts download code13 → Miscellaneous tidying of artifacts download code

Comment hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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 hidden (mozreview-request)

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

Mike Hommey [:glandium]

Assignee

Updated

•

7 years ago

Blocks: 1355731

Mike Hommey [:glandium]

Assignee

Comment 4

•

7 years ago

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)

Mike Hommey [:glandium]

Assignee

Comment 5

•

7 years ago

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)

Chris Manchester (limited bugmail, email directly)

Comment 6

•

7 years ago

mozreview-review

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

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

Attachment #8857361 - Flags: review?(cmanchester) → review+

Chris Manchester (limited bugmail, email directly)

Comment 7

•

7 years ago

mozreview-review

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

Attachment #8857362 - Flags: review?(cmanchester) → review+

Mike Hommey [:glandium]

Assignee

Updated

•

7 years ago

Depends on: 1356064

Mike Hommey [:glandium]

Assignee

Comment 8

•

7 years ago

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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/

Mike Hommey [:glandium]

Assignee

Comment 12

•

7 years ago

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

Mike Hommey [:glandium]

Assignee

Comment 13

•

7 years ago

Err, the unit test fails randomly on automation.

Comment hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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/

Mike Hommey [:glandium]

Assignee

Updated

•

7 years ago

Depends on: 1356101

Comment hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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/

Chris Manchester (limited bugmail, email directly)

Comment 20

•

7 years ago

mozreview-review

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

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

Attachment #8857363 - Flags: review?(cmanchester) → review+

Comment hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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/

Pulsebot

Comment 24

•

7 years ago

Pushed by mh@glandium.org:
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

Iris Hsiao [:ihsiao]

Comment 25

•

7 years ago

bugherder

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

Status: NEW → RESOLVED

Closed: 7 years ago

status-firefox55: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla55

BMO Automation

Updated

•

6 years ago

Product: Core → Firefox Build System

You need to log in before you can comment on or make changes to this bug.