Code review comment for lp:~vila/bzr/186920-lp-proxy

>>>>> "jam" == John A Meinel <email address hidden> writes:

    jam> +class Transport(xmlrpclib.Transport):
    jam> +
    jam> + def __init__(self, scheme, use_datetime=0):

    jam> ^- Shouldn't we have our own name here? We *can* re-use
    jam> the existing name, but it is potentially confusing.

    jam> Especially given that we have a "Transport" hierarchy
    jam> already (and not to mention that so does Paramiko), I'd
    jam> like to at least call it "XMLRPCTransport".

Sure, done.

    jam> Also, it seems that Gordon responded that there was a
    jam> problem with not passing the original port to the proxy?
    jam> Did you fix that in this? Or did his comment come after
    jam> the merge proposal?

His comment went after the mp, I've fixed it since so I'll
resubmit.

    jam> I assume that the http/https trick is this line:

    jam> + self._opener = _urllib2_wrappers.Opener()

    jam> (Opener supposedly creates the right instance of HTTP or HTTPS based on
    jam> the url/the url of the proxy?) I assume that is why you don't need:
    jam> - if uri_type == 'https':
    jam> - transport = xmlrpclib.SafeTransport()
    jam> - else:
    jam> - transport = xmlrpclib.Transport()
    jam> + transport = Transport(uri_type)

    jam> anymore.

Correct. I could have refactored even more aggressively (to pass
the full service url and set the agent) but I wanted to keep the
patch as minimal as possible.

        Vincent