--installer-url and --test-url should accept local files

RESOLVED FIXED

Status

Release Engineering
Mozharness
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
I'm running mozharness locally and want to make it easier to point it at installers/tests generated from local builds. Patch will be uploaded momentarily.

Comment 1

4 years ago
--installer-path ?
We don't have an equivalent for --test-zip-path but I don't think it would be difficult to add.
(Assignee)

Comment 2

4 years ago
Created attachment 8452626 [details] [diff] [review]
Accept local paths to --installer-url and --test-url

If the argument given to --installer-url and --test-url looks like a
path (we only support UNIX paths), we prepend file:// to the value. The
existing URL handling code transparently "downloads" these URLs by
copying the file.

As part of testing this, I realized my local build wasn't producing a
jsshell archive and failed "downloads" would retry. I added some logic
to the download error handling logic to detect local filesystem errors
so this error is treated as fatal.

The .errno comparison should probably catch a few more failures, such as
permissions failures. But I'm not sure which ones are exclusive to
file:// vs other protocols. I suppose we could have certain protocol
handlers not retry. But this was the simplest patch.
Attachment #8452626 - Flags: review?(aki)
(Assignee)

Updated

4 years ago
Assignee: nobody → gps
Status: NEW → ASSIGNED
(Assignee)

Comment 3

4 years ago
(In reply to Aki Sasaki [:aki] from comment #1)
> --installer-path ?
> We don't have an equivalent for --test-zip-path but I don't think it would
> be difficult to add.

Well, I tried --installer-path with the code path I was using and it didn't like it! I think there are differences between --installer-path and --installer-url. I didn't want to track those down.

I think computers should take the cognitive load of "url" vs "path" away from the user. IMO we should trend towards a single argument (--installer/--tests-zip) that accepts either both URLs and paths. This patch gets us closer to that.

Comment 4

4 years ago
Comment on attachment 8452626 [details] [diff] [review]
Accept local paths to --installer-url and --test-url

Hm, I thought file:// didn't work, but now it does.  Not sure if this was faulty testing back then or a change in urllib2.
Attachment #8452626 - Flags: review?(aki) → review+

Comment 6

4 years ago
pyflakes sez:
mozharness/base/script.py:208: undefined name 'errno'
Merged to production branch of mozharness.
Is comment 6 a problem? Do we need to pull it out?
Flags: needinfo?(gps)
(Assignee)

Comment 9

4 years ago
It's not a problem for automation use - only local use. I pushed a trivial followup (missing import of a stdlib module).

I keep forgetting I don't have my Mercurial extension for running pyflakes on commit globally installed. I need to fix that...
Flags: needinfo?(gps)
Merged to production, and deployed.
(Assignee)

Comment 11

4 years ago
If this is deployed to production, why is the bug still open?

Comment 12

4 years ago
We leave it to the owner to resolve, since it's not always clear whether a bug is closable after the initial set of patches is landed.
(Assignee)

Comment 13

4 years ago
Ahh, didn't know that.

In that case...
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Comment 14

4 years ago
mozharness/base/script.py:215: [E, ScriptMixin._download_file] Module 'errno' has no 'NOENT' member

https://docs.python.org/2/library/errno.html

Did you mean errno.ENOENT ?

Comment 16

4 years ago
In production.
You need to log in before you can comment on or make changes to this bug.