Martin Langhoff wrote: > On Sat, Feb 14, 2009 at 9:11 AM, Simon Schampijer <si...@schampijer.de> wrote: >> Please find attached the patch against master. > > Looks good to me (but I know nothing of what's changed in master...) > >> - i use the backup_url to see if we are associated with a schoolserver >> - why did you use the jabber server for this 'xs_fqdn = prof.jabber_server'? > > Good question. Neither is the right one. In a XS driven net, both are > equal. In a XS-on-the-internet situation, the "public XS" may decide > to not offer backup service. Of the 3 (moodle/webapps, xmpp, backup), > backup is the most burdensome on the server. > > So I think there is a (very marginal) advantage to using the jabber > server. But the most important hting is that 0.82.x and master use the > same, so whatever you do, both should use the same...
Our registration URL is REGISTER_URL = 'http://schoolserver:8080/', wouldn't the right Domain than be 'schoolserver'? Since the cookie is about the registration with the schoolserver this makes most sense to me (the jabber server could be somewhere else). > (The right fix is to have a 'schoolserver fqdn' entry in the > profile... but that's for the next Sugar dev cycle I guess...) > >> - c.execute('''CREATE TABLE IF NOT EXISTS >> + moz_cookies >> + (id INTEGER PRIMARY KEY, >> + name TEXT, >> + value TEXT, >> + host TEXT, >> + path TEXT, >> + expiry INTEGER, >> + lastAccessed INTEGER, >> + isSecure INTEGER, >> + isHttpOnly INTEGER);''') >> >> - is the ';' correct here or a typo? > > typo ok; >> - i only except for sqlite3.Error > > Is that the only thing that could go wrong? My thinking has been: if > we fail, let the startup succeed. This is a good feature, but not a > showstopper. > >> - what bothers me a bit is that you don't get an error when the database >> does not exist - sqlite creates a new one actually - so we might return as >> well on 'if not os.path.exists(os.path.join(_profile_path, >> 'cookies.sqlite'))' Well, all the calls in the try block are sqlite3 ones - if they fail - we catch it. If something else goes wrong - we want to fail and not hide ;p > The DB does not exist on the first use of Browse. Actually, it does > not get created until the first website sets the first cookie, AFAICS. > > That means that on the first use of Browse the user goes to the XS and > doesn't get autenticated. So if the DB doesn't exist, _we want to > create it_. It's not a failure, it's success. Ok, sounds good. >> - the method could even be a function as it does not interact at all with >> the class itself, not sure what is nicer > > I'd prefer a function, but it's not my codebase, so follow the style... :-) Done. BTW: Is there a spec you used for the cookie format? I find field descriptions like expires - you name it expiry. Thanks, Simon _______________________________________________ Devel mailing list Devel@lists.laptop.org http://lists.laptop.org/listinfo/devel