[Bugs] #2289 UNSP: sugar freezes when register widget is clicked (if Server is unresponsive)
Sugar Labs Bugs
bugtracker-noreply at sugarlabs.org
Sun Sep 5 07:18:53 EDT 2010
#2289: sugar freezes when register widget is clicked (if Server is unresponsive)
------------------------------------------+---------------------------------
Reporter: lfaraone | Owner: tomeu
Type: defect | Status: new
Priority: Unspecified by Maintainer | Milestone: Unspecified by Release Team
Component: sugar | Version: Unspecified
Severity: Major | Keywords: r?
Distribution: Ubuntu | Status_field: Unconfirmed
------------------------------------------+---------------------------------
Comment(by sascha_silbe):
If you post the patch to the mailing list (using git-send-email) you'll
have a better chance if it getting reviewed soon (it will happen in Trac
as well, but might take more time).
Some things I noticed while giving the patch a quick look:
* subject is not really useful
* description is missing
* non-bugfix changes mixed in (what's the reason for the string import
change anyway?)
* change descriptions ("New class [...]") belong in the commit
description, not the source code
* please consider adding docstrings to the new classes (but only if you
come up with something that's more descriptive than the name itself)
* if all we're doing in {{{TimeoutServerProxy}}} is adding a parameter and
we only have one trivial caller, why add a new class at all?
{{{
+ def make_connection(self, host):
+ host, extra_headers, x509 = self.get_host_info(host)
+ conn = TimeoutHTTP(host, timeout=self.timeout)
+ return conn
}}}
Please combine the last two lines. The variable name doesn't add any
information as it should be clear from the method name that the return
value will be a connection (and if it isn't clear enough, a docstring
would be better).
While this the timeout a stopgap to prevent Sugar from hanging
indefinitely, a proper fix will additionally need to make the registration
asynchronous. It might be too late to ship the full fix in 0.90, though.
--
Ticket URL: <http://bugs.sugarlabs.org/ticket/2289#comment:3>
Sugar Labs <http://sugarlabs.org/>
Sugar Labs bug tracking system
More information about the Bugs
mailing list