Re: [Sugar-devel] Patch review request for ticket #2290
Excerpts from Marco Pesenti Gritti's message of Tue Sep 07 01:29:30 +0200 2010: So many reasons to move to introspection :) Then someone should get started on packaging the introspection stuff for Debian, Ubuntu and if necessary Fedora (the distributions supported by sugar-jhbuild). :-P ;) Sascha -- http://sascha.silbe.org/ http://www.infra-silbe.de/ signature.asc Description: PGP signature ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel
Re: [Sugar-devel] Patch review request for ticket #2290
On Tue, Sep 7, 2010 at 13:53, Sascha Silbe sascha-ml-reply-to-201...@silbe.org wrote: Excerpts from Marco Pesenti Gritti's message of Tue Sep 07 01:29:30 +0200 2010: So many reasons to move to introspection :) Then someone should get started on packaging the introspection stuff for Debian, Ubuntu and if necessary Fedora (the distributions supported by sugar-jhbuild). :-P ;) I'm not too worried about it being packaged in those distros, it only may not get to the stable/testing release of your choice :( Regards, Tomeu Sascha -- http://sascha.silbe.org/ http://www.infra-silbe.de/ ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel
Re: [Sugar-devel] Patch review request for ticket #2290
On Sun, Sep 5, 2010 at 3:44 PM, Dipankar Patro dipan...@seeta.in wrote: Hi, In reference to ticket #2290 (http://bugs.sugarlabs.org/ticket/2290) I have uploaded a patch (have attached it too). Problem: in the register procedure the time out of connection was not implemented, for an unavailable server. Solution in Patch: - Have implemented a TimedOut connection (TimedOutServerProxy( ) function) which encounters the problem of unavailable servers. - The connection timeout can be changed in seconds through the variable : 'timeout'. The default setting is at : timeout=socket._GLOBAL_DEFAULT_TIMEOUT Change it to : timeout=10, to find Sugar does not freeze anymore. How long is the default timeout? Are we freezing because we are doing the xmlrpc requests synchronously? Marco ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel
Re: [Sugar-devel] Patch review request for ticket #2290
On Mon, Sep 6, 2010 at 11:51, Marco Pesenti Gritti ma...@marcopg.org wrote: On Sun, Sep 5, 2010 at 3:44 PM, Dipankar Patro dipan...@seeta.in wrote: Hi, In reference to ticket #2290 (http://bugs.sugarlabs.org/ticket/2290) I have uploaded a patch (have attached it too). Problem: in the register procedure the time out of connection was not implemented, for an unavailable server. Solution in Patch: - Have implemented a TimedOut connection (TimedOutServerProxy( ) function) which encounters the problem of unavailable servers. - The connection timeout can be changed in seconds through the variable : 'timeout'. The default setting is at : timeout=socket._GLOBAL_DEFAULT_TIMEOUT Change it to : timeout=10, to find Sugar does not freeze anymore. How long is the default timeout? Are we freezing because we are doing the xmlrpc requests synchronously? Previous discussion is actually in http://bugs.sugarlabs.org/ticket/2289 and not in #2290. Looks like we cannot do XML-RPC with GIO because it doesn't support POST for http requests. The right component in our stack would be libsoup but we find again that we need introspection to use it because nobody cared to write, maintain and package python bindings for it. This makes xmlrpclib usage async, but it also uses private API: http://www.mail-archive.com/py...@daa.com.au/msg12971.html So maybe the best we can do for now is indeed setting a timeout. Regards, Tomeu Marco ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel
Re: [Sugar-devel] Patch review request for ticket #2290
Sorry for that duplicate bug. Missed out that lfaraone already filed the bug. I have attached the revised patch. (uploaded at bugs.sl.o too) * changed things (subject, description, etc) according to Sascha Silbe's suggestions. @ Marco : The default timeout is way too long (unable to find out exact time). Yes, the process is synchronous, thats why Sugar is freezing. @ Tomeu: Thanks for the review. Will try to get the registration process asynchronous. Regards, Dipankar On Mon, Sep 6, 2010 at 4:12 PM, Tomeu Vizoso to...@sugarlabs.org wrote: On Mon, Sep 6, 2010 at 11:51, Marco Pesenti Gritti ma...@marcopg.org wrote: On Sun, Sep 5, 2010 at 3:44 PM, Dipankar Patro dipan...@seeta.in wrote: Hi, In reference to ticket #2290 (http://bugs.sugarlabs.org/ticket/2290) I have uploaded a patch (have attached it too). Problem: in the register procedure the time out of connection was not implemented, for an unavailable server. Solution in Patch: - Have implemented a TimedOut connection (TimedOutServerProxy( ) function) which encounters the problem of unavailable servers. - The connection timeout can be changed in seconds through the variable : 'timeout'. The default setting is at : timeout=socket._GLOBAL_DEFAULT_TIMEOUT Change it to : timeout=10, to find Sugar does not freeze anymore. How long is the default timeout? Are we freezing because we are doing the xmlrpc requests synchronously? Previous discussion is actually in http://bugs.sugarlabs.org/ticket/2289 and not in #2290. Looks like we cannot do XML-RPC with GIO because it doesn't support POST for http requests. The right component in our stack would be libsoup but we find again that we need introspection to use it because nobody cared to write, maintain and package python bindings for it. This makes xmlrpclib usage async, but it also uses private API: http://www.mail-archive.com/py...@daa.com.au/msg12971.html So maybe the best we can do for now is indeed setting a timeout. Regards, Tomeu Marco ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel From a2cf4863eca617bcd57008730e13ab8af95e7cfa Mon Sep 17 00:00:00 2001 From: Dipankar Patro dipan...@seeta.in Date: Mon, 6 Sep 2010 21:42:51 +0530 Subject: [PATCH] Register widget click event reflects a timeout on unavailability of servers [Ticket #2289] The register widget when clicked, used to cause sugar to freeze. Added a timeout facilitated ServerProxy() so that connection to schoolserver is dropped in case it is not available. --- src/jarabe/desktop/schoolserver.py | 28 ++-- 1 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/jarabe/desktop/schoolserver.py b/src/jarabe/desktop/schoolserver.py index 62519df..e60e26f 100644 --- a/src/jarabe/desktop/schoolserver.py +++ b/src/jarabe/desktop/schoolserver.py @@ -16,7 +16,8 @@ import logging from gettext import gettext as _ -from xmlrpclib import ServerProxy, Error +import httplib +import xmlrpclib import socket import os import string @@ -76,6 +77,29 @@ def store_identifiers(serial_number, uuid, backup_url): class RegisterError(Exception): pass +class TimeoutHTTP(httplib.HTTP): + def __init__(self, host='', port=None, strict=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT): +if port == 0: +port = None +self._setup(self._connection_class(host, port, strict, timeout)) + +class TimeoutTransport(xmlrpclib.Transport): +def __init__(self, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, *args, **kwargs): +xmlrpclib.Transport.__init__(self, *args, **kwargs) +self.timeout = timeout + +def make_connection(self, host): +host, extra_headers, x509 = self.get_host_info(host) +return TimeoutHTTP(host, timeout=self.timeout) + +class TimeoutServerProxy(xmlrpclib.ServerProxy): + Creates a server proxy with Timeout facility. + Timeout sent in argument can be used to drop connection + for unavailable servers +def __init__(self, url, timeout, *args, **kwargs): +kwargs['transport'] = TimeoutTransport(timeout=timeout, use_datetime=kwargs.get('use_datetime', 0)) +xmlrpclib.ServerProxy.__init__(self, url, *args, **kwargs) + def register_laptop(url=REGISTER_URL): profile = get_profile() @@ -95,7 +119,7 @@ def register_laptop(url=REGISTER_URL): nick = client.get_string('/desktop/sugar/user/nick') -server = ServerProxy(url) +server = TimeoutServerProxy(url,10) try: data = server.register(sn, nick, uuid_, profile.pubkey) except (Error, socket.error): -- 1.7.0.4 ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel
Re: [Sugar-devel] Patch review request for ticket #2290
Excerpts from Dipankar Patro's message of Mon Sep 06 19:04:04 +0200 2010: I have attached the revised patch. (uploaded at bugs.sl.o too) Please use git send-email to send patches so they appear as inline text rather than attachments. With many email clients it's hard to reply to text attachments. I still think class TimeoutServerProxy could be eliminated and that this change would make the code simpler and easier to understand. * changed things (subject, description, etc) according to Sascha Silbe's suggestions. Thanks! Let me suggest some further improvements: Subject: [PATCH] Register widget click event reflects a timeout on unavailability of servers Time out on registration process to prevent indefinite UI hang (SL#2289) [Ticket #2289] The convention so far was to append the ticket number to the subject so it appears in one-line commit logs. Mentioning it separately is OK, but if you're already changing the summary and description it makes sense to adjust it. The register widget when clicked, used to cause sugar to freeze. Added a timeout facilitated ServerProxy() so that connection to schoolserver is dropped in case it is not available. Registration with the school server is currently done synchronously. To prevent the UI from hanging indefinitely if the school server is reachable but unresponsive we add an explicit timeout. As you can see it's still not perfect, but hopefully conveys the rationale behind the patch and the high-level changes it makes better. The focus in patch descriptions is on what the effect of the change is and why it is done, not so much how it is achieved (that's visible in the patch itself). Choosing a good description is hard, even after years of training. But it makes life a lot easier if you're trying to understand some piece of code months or years later. Give git blame some file and git log commit ID^..commit ID a try sometime to see how to use the commit log to understand some code. (There are probably even GUI tools for that, but as I'm a console freak myself I don't know about any). @ Marco : The default timeout is way too long (unable to find out exact time). Yes, the process is synchronous, thats why Sugar is freezing. The chosen timeout of 10 seconds seems sensible to me, FWIW. It's long enough to allow a busy server (even a remote one) to answer and short enough so the user doesn't give up and reset the machine. Sascha -- http://sascha.silbe.org/ http://www.infra-silbe.de/ signature.asc Description: PGP signature ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel
Re: [Sugar-devel] Patch review request for ticket #2290
On Mon, Sep 6, 2010 at 6:04 PM, Dipankar Patro dipan...@seeta.in wrote: Sorry for that duplicate bug. Missed out that lfaraone already filed the bug. I have attached the revised patch. (uploaded at bugs.sl.o too) * changed things (subject, description, etc) according to Sascha Silbe's suggestions. @ Marco : The default timeout is way too long (unable to find out exact time). Yes, the process is synchronous, thats why Sugar is freezing. With Tomeu clarification the very short timeout make sense. As Sascha suggested the log should explain why we are doing this though. I think it would be good to also put a similar comment in a FIXME in the code. Marco ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel
Re: [Sugar-devel] Patch review request for ticket #2290
On Mon, Sep 6, 2010 at 11:42 AM, Tomeu Vizoso to...@sugarlabs.org wrote: Previous discussion is actually in http://bugs.sugarlabs.org/ticket/2289 and not in #2290. Looks like we cannot do XML-RPC with GIO because it doesn't support POST for http requests. The right component in our stack would be libsoup but we find again that we need introspection to use it because nobody cared to write, maintain and package python bindings for it. So many reasons to move to introspection :) Marco ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel
[Sugar-devel] Patch review request for ticket #2290
Hi, In reference to ticket #2290 (http://bugs.sugarlabs.org/ticket/2290) I have uploaded a patch (have attached it too). Problem: in the register procedure the time out of connection was not implemented, for an unavailable server. Solution in Patch: - Have implemented a TimedOut connection (TimedOutServerProxy( ) function) which encounters the problem of unavailable servers. - The connection timeout can be changed in seconds through the variable : 'timeout'. The default setting is at : timeout=socket._GLOBAL_DEFAULT_TIMEOUT Change it to : timeout=10, to find Sugar does not freeze anymore. Regards, Dipankar From e636b25da0973e9986b6fcd2a7f0f5a48d3e0265 Mon Sep 17 00:00:00 2001 From: Dipankar Patro dipan...@seeta.in Date: Sat, 4 Sep 2010 19:53:54 +0530 Subject: [PATCH] Register Bug Solution: ticket #2290 --- src/jarabe/desktop/schoolserver.py | 40 ++- 1 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/jarabe/desktop/schoolserver.py b/src/jarabe/desktop/schoolserver.py index 62519df..58ffb88 100644 --- a/src/jarabe/desktop/schoolserver.py +++ b/src/jarabe/desktop/schoolserver.py @@ -16,10 +16,12 @@ import logging from gettext import gettext as _ -from xmlrpclib import ServerProxy, Error + +import httplib +import xmlrpclib import socket import os -import string +from string import ascii_uppercase import random import time import uuid @@ -37,8 +39,8 @@ def generate_serial_number(): serial_part1 = [] -for y in range(3) : -serial_part1.append(random.choice(string.ascii_uppercase)) +for y_ in range(3) : +serial_part1.append(random.choice(ascii_uppercase)) serial_part1 = ''.join(serial_part1) serial_part2 = str(int(time.time()))[-8:] @@ -76,6 +78,30 @@ def store_identifiers(serial_number, uuid, backup_url): class RegisterError(Exception): pass +# New class TimeoutServerProxy to implement timeout controlled connection, derived from xmlrpclib.ServerProxy() +# LP Bug #617813 +class TimeoutHTTP(httplib.HTTP): + def __init__(self, host='', port=None, strict=None, +timeout=socket._GLOBAL_DEFAULT_TIMEOUT): +if port == 0: +port = None +self._setup(self._connection_class(host, port, strict, timeout)) + +class TimeoutTransport(xmlrpclib.Transport): +def __init__(self, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, *args, **kwargs): +xmlrpclib.Transport.__init__(self, *args, **kwargs) +self.timeout = timeout + +def make_connection(self, host): +host, extra_headers, x509 = self.get_host_info(host) +conn = TimeoutHTTP(host, timeout=self.timeout) +return conn + +class TimeoutServerProxy(xmlrpclib.ServerProxy): +def __init__(self, url, timeout, *args, **kwargs): +kwargs['transport'] = TimeoutTransport(timeout=timeout, use_datetime=kwargs.get('use_datetime', 0)) +xmlrpclib.ServerProxy.__init__(self, url, *args, **kwargs) + def register_laptop(url=REGISTER_URL): profile = get_profile() @@ -89,13 +115,15 @@ def register_laptop(url=REGISTER_URL): else: sn = generate_serial_number() uuid_ = str(uuid.uuid1()) -jabber_server = client.get_string('/desktop/sugar/collaboration/jabber_server') +setting_name = '/desktop/sugar/collaboration/jabber_server' +jabber_server = client.get_string(setting_name) store_identifiers(sn, uuid_, jabber_server) url = 'http://' + jabber_server + ':8080/' nick = client.get_string('/desktop/sugar/user/nick') -server = ServerProxy(url) +server = TimeoutServerProxy(url) + try: data = server.register(sn, nick, uuid_, profile.pubkey) except (Error, socket.error): -- 1.7.0.4 ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel