899221 - fix mozprofile proxy API and documentation

Status: RESOLVED WONTFIX

(Testing :: Mozbase, defect)

Description

[Jeff Hammel]

`The thing about refactoring an API is to make sure the new API is no
worse than the existing...`

=> The Problem:

mozprofile proxy is hard to understand, documented wrongly, and is
possibly wrong.

The mozprofile.Profile ctor gives the following for proxy:

https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/profile.py#L39

 :param proxy: setup a proxy - dict of server-loc,server-port,ssl-port

However, this is incorrect.  This is passed to permissions.Permissions.network_prefs:

https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/permissions.py#L272

...which trickles down into pac_prefs.  In fact, the only place you
can see what the parameters are supposed to be is in the template:

https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/permissions.py#L321

"""
...
if (isHttp) return 'PROXY %(remote)s:%(http)s';
if (isHttps) return 'PROXY %(remote)s:%(https)s';
if (isWebSocket) return 'PROXY %(remote)s:%(ws)s';
if (isWebSocketSSL) return 'PROXY %(remote)s:%(wss)s';
"""

To find these values, look for DEFAULT_PORTS at the top of the file
and follow the link
http://hg.mozilla.org/mozilla-central/file/b871dfb2186f/build/automation.py.in#l28
:

Of course, this mentions nothing of wss (websockets over SSL).

I'm not sure why we except ValueErrors here, that would be nice to
comment on:

https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/permissions.py#L188


See also https://bugzilla.mozilla.org/show_bug.cgi?id=688667#c66 ; in
automation, if we provide wss with its own port....AllTheThings break!
I don't understand the reason for this disparity, but in bug 688667 I
blindly followed legacy.


=> My Recommendations:

* fix the signatures of at least mozprofile.permissions.pac_prefs and
  .network_prefs to use the variables http, https, ws, and wss vs
  passing an undocumnted dictionary

* better documentation and comments, including correcting the
  erroneous documentation of the Profile docstring (and ideally better
  end-to-end documentation)

* don't reference the legacy automation.py.in as canon, since the goal
  is to switch canons.  If we must reference it, independently explain
  what we are doing

* and, probably most importantly, sort out the wss vs https difference
  of automation.py.in vs mozprofile.permissions

Comment 1

[Jonathan Griffin (:jgriffin)]

+1.  Let's see if we can tackle this during the work week.

Comment 2

[Jeff Hammel]

(In reply to Jonathan Griffin (:jgriffin) from comment #1)
> +1.  Let's see if we can tackle this during the work week.

This didn't happen.  Maybe we can figure out when/who to do this next mozbase meeting.  Or volunteers welcome :)

Comment 3

[Geoff Brown [:gbrown]]

Mass closing bugs with no activity in 2+ years. If this bug is important to you, please re-open.