Jeff Hammel Assignee
	Description • 11 years ago

From https://bugzilla.mozilla.org/show_bug.cgi?id=830430#c15 :

"mozprofile should be modified to read e.g. prefs.js from the web."

For any place where we read preferences in
https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/prefs.py
we should use (sadly) urllib2 to read from the web if its a URL, and
`file` to read if, as is now, it is a local path (no scheme://) or a
file:// url.

A test should be introduced using mozhttpd to serve a profile.

	Jeff Hammel Assignee
	Comment 1 • 11 years ago

Ideally this can be done with a decorator or other way that requires as little code as possible.

I also wonder, since reading wrt either a local file or a url, if we should add a utility function to mozfile to this end.  I think this is the way to go

	Jeff Hammel Assignee
	Comment 2 • 11 years ago

I'll work on this unless someone else wants it

	Jeff Hammel Assignee
	Comment 3 • 11 years ago

I've decided that putting this in mozfile straight-off and releasing a new version and depending on it is the way to go.

	Jeff Hammel Assignee
	Comment 4 • 11 years ago

this modifies the prefs.Preferences.read_* methods to use mozfile.load and adds a smokescreen test for such.  It is unclear to me how this will look in practice without porting one of the out-of-m-c harnesses or the like to this (mozregression might be a good choice, or mozmill; not sure apropos the status of if bugs should be filed for all-the-things).

Looking at the structure of prefs.py, some minor refactoring may be warranted if not necessary to fulfill the intent of this bug: namely, to support using default preferences from mozilla-central for harnesses and tools not in mozilla-central.  My suggestions:
  - add should take a path string and add via read() and deprecate add_file
  - __iadd__ alias to add
  - read should be fixed:
    - currently it does not handle the normal prefs.js case; it should!
    - it should try initially the method that matches the file/url
      extension
    - for this reason and other extensibility, the readers should be classes
    - do we want to support .ini format? i don't know of a use-case
I will file a follow up for this.

Obviously more code may need to change when we're actually ready to make the shift.

	Jeff Hammel Assignee
	Comment 5 • 11 years ago

(In reply to Jeff Hammel [:jhammel] from comment #4)
> Created attachment 748328 [details] [diff] [review]
> read prefs ttw
> 
> this modifies the prefs.Preferences.read_* methods to use mozfile.load and
> adds a smokescreen test for such.  It is unclear to me how this will look in
> practice without porting one of the out-of-m-c harnesses or the like to this
> (mozregression might be a good choice, or mozmill; not sure apropos the
> status of if bugs should be filed for all-the-things).
> 
> Looking at the structure of prefs.py, some minor refactoring may be
> warranted if not necessary to fulfill the intent of this bug: namely, to
> support using default preferences from mozilla-central for harnesses and
> tools not in mozilla-central.  My suggestions:
>   - add should take a path string and add via read() and deprecate add_file
>   - __iadd__ alias to add
>   - read should be fixed:
>     - currently it does not handle the normal prefs.js case; it should!
>     - it should try initially the method that matches the file/url
>       extension
>     - for this reason and other extensibility, the readers should be classes
>     - do we want to support .ini format? i don't know of a use-case
> I will file a follow up for this.
> 
> Obviously more code may need to change when we're actually ready to make the
> shift.

follow up filed: https://bugzilla.mozilla.org/show_bug.cgi?id=871071

	Andrew Halberstadt [:ahal]
	Comment 6 • 11 years ago

Comment on attachment 748328 [details] [diff] [review]
read prefs ttw

Review of attachment 748328 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. I can't remember, are we supposed to version bump when adding a dependency?

	Jeff Hammel Assignee
	Comment 7 • 11 years ago

(In reply to Andrew Halberstadt [:ahal] from comment #6)
> Comment on attachment 748328 [details] [diff] [review]
> read prefs ttw
> 
> Review of attachment 748328 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. I can't remember, are we supposed to version bump when adding a
> dependency?

If the code is at a release state, or IMHO when in doubt, yes.  Version bumping never hurts, really.

	Jeff Hammel Assignee
	Comment 8 • 11 years ago

Also: will change the dependency to be 'mozfile >= 0.6' on checkin now that it is released

	Jeff Hammel Assignee
	Comment 9 • 11 years ago

pushed: https://github.com/mozilla/mozbase/commit/4a38d2c16da09d5feb571eac6bdbdb853ac8b442

will bump mozprofile in bug 871034 and close both this and that

	Jeff Hammel Assignee
	Comment 10 • 11 years ago

done; follow-up of note at bug 871071