Download firefox from the firefox-download task #21194
Conversation
jgraham
force-pushed
the
fx_task_download
branch
9 times, most recently
from
965cbc3
to
e2e4235
Compare
|
@Hexcles I think you might be the correct reviewer for this, but feel free to punt to someone else if not. |
|
Azure Pipelines failed, which seems to be related to shallow clone. Perhaps you could rebase this and try again? |
tools/ci/run_tc.py
Outdated
| def download_url_to_descriptor(fd, url, max_retries=3): | ||
| """Download an URL in chunks and saves it to a file descriptor (truncating it) | ||
| It doesn't close the descriptor, but flushes it on success. |
There was a problem hiding this comment.
Nit: add a blank line before this line, and remove 1 level of indentation.
There was a problem hiding this comment.
An initial blank line doesn't match the style of other code afaict?
tools/ci/tc/tasks/test.yml
Outdated
| @@ -82,6 +82,11 @@ components: | |||
| browser-firefox: | |||
| depends-on: | |||
| - download-firefox-${vars.channel} | |||
| download-artifacts: | |||
| - task: download-firefox-${vars.channel} | |||
| artifact: public/results/firefox-${vars.channel}.* | |||
There was a problem hiding this comment.
I'd suggest using a different name here. Perhaps artifact-glob, or just glob? There's so many artifact* in download_artifacts that it's a bit confusing.
Alternatively, could we put a concrete filename here instead of a pattern? It'd simplify download_artifacts, too.
There was a problem hiding this comment.
A concrete filename ends up being stronger coupling that causes problems if e.g. the archive format for a download changes. So that's why I went with a glob pattern.
There was a problem hiding this comment.
I was hoping that we could populate this field in the download task after we download the browser, but it seems like plumbing would be messy?
There was a problem hiding this comment.
Yeah, I'd rather leave that as a future enhancement.
This adds a download-artifacts key to the task definition which allows specifying a list of artifacts to be downloaded from other tasks, and a destination. Archives may be automatically unpacked so the content can be used directly in the rest of the task.
This can be used to remove the version number and so give the download a consistent name
This ensures that all the runs for a specific revision get the exact same version of Firefox
jgraham
force-pushed
the
fx_task_download
branch
from
e2e4235
to
54feb0f
Compare
| def download_url_to_descriptor(fd, url, max_retries=3): | ||
| """Download an URL in chunks and saves it to a file descriptor (truncating it) | ||
| It doesn't close the descriptor, but flushes it on success. |
There was a problem hiding this comment.
| It doesn't close the descriptor, but flushes it on success. | |
| It doesn't close the descriptor, but flushes it on success. |
I meant having a blank line between the one-line summary and the details, like this, which seems to match the coding style in e.g. browser.py. But anyway, this is really just nitpicking/optional :)
This allows using the same revision of Firefox for all tasks in a taskgroup.