Re: [Sugar-devel] Patch review request for ticket #2290

2010-09-07 Thread Sascha Silbe
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

2010-09-07 Thread Tomeu Vizoso
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

2010-09-06 Thread Marco Pesenti Gritti
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

2010-09-06 Thread Tomeu Vizoso
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

2010-09-06 Thread Dipankar Patro
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

2010-09-06 Thread Sascha Silbe
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

2010-09-06 Thread Marco Pesenti Gritti
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

2010-09-06 Thread Marco Pesenti Gritti
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

2010-09-05 Thread Dipankar Patro
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