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



Release Engineering
4 years ago
4 years ago


(Reporter: gps, Assigned: gps)


Firefox Tracking Flags

(Not tracked)



(1 attachment)



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.

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)


4 years ago
Assignee: nobody → gps

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)

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.

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.

Comment 13

4 years ago
Ahh, didn't know that.

In that case...
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


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.