[Sugar-devel] [PATCH sugar] Register with schoolserver: adopt to changes in xmlrpclib for python 2.7 OLPC #10776

[Simon Schampijer]

On 05/23/2011 10:01 AM, Simon Schampijer wrote:
> On 05/20/2011 10:15 PM, Sascha Silbe wrote:
>> Excerpts from Simon Schampijer's message of Thu May 19 16:08:36 +0200
>> 2011:
>>
>>
>> s/adopt/adapt/ (or adjust)
>>
>>> Python 2.7 switched from using httplib.HTTP to using
>>> httplib.HTTPConnection,
>>> as the httplib.HTTPConnection includes a timeout by default we can use a
>>> xmlrpclib.Transport directly and do not need to subclass it.
>>
>> I guess we don't need to subclass Transport, but we probably need to
>> call socket.setdefaulttimeout() during Sugar start-up:
>>
>>>> socket.setdefaulttimeout(timeout)
>>
>>>> Set the default timeout in floating seconds for new socket objects.
>>>> A value of None indicates that new socket objects have no timeout.
>>>> When the socket module is first imported, the default is None.
>
> Hmm, yes sounds about right. I don't think we have to do it during sugar
> startup (not sure about other side-effects), just after the 'else' is
> fine. In any case I guess the real solution is to make the registration
> asynchronous: http://bugs.sugarlabs.org/ticket/2289#comment:4 but I did
> not want to block on it.
>
>> [src/jarabe/desktop/schoolserver.py]
>>> - server = xmlrpclib.ServerProxy(url, _TimeoutTransport())
>>> + if sys.version_info[1]< 7:
>>
>> "if sys.hexversion< 0x02070000:" has a higher chance of continuing to
>> work after a transition to Py3K. The 2to3 tool already takes care of
>> renaming xmlrpclib to xmlrpc.client [1].
>
> I guess you mean: "sys.hexversion < 0x2070000". Is ok to me.
>
>>> + server = xmlrpclib.ServerProxy(url, _TimeoutTransport())
>>> + else:
>>> + server = xmlrpclib.ServerProxy(url, xmlrpclib.Transport())
>>
>> We can skip the transport altogether for 2.7+. That way we support
>> https, too (SafeTransport instead of Transport).
>
> Sounds right: "The optional second argument is a transport factory
> instance; by default it is an internal SafeTransport instance for https:
> URLs and an internal HTTP Transport instance otherwise."

I have attached a new patch with the changes noted here and tested with 
a school server.

Regards,
    Simon