| thumper | trivial review for someone: https://code.edge.launchpad.net/~thumper/launchpad/vcs-imports-permission-review/+merge/25867 | 06:05 |
|---|---|---|
| === intellectronica changed the topic of #launchpad-reviews to: On Call: intellectronica || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adiroiban | intellectronica: hi. is there anything I need to do for having this branch landed ? https://code.edge.launchpad.net/~adiroiban/launchpad/bug-570899/+merge/25443 | 13:49 |
| intellectronica | adiroiban: no, i'll submit it for tests and landing on your behalf | 13:53 |
| leonardr | intellectronica: i have a long (850 lines) but hopefully stimulating branch for you to review | 15:21 |
| leonardr | https://code.edge.launchpad.net/~leonardr/lazr.restful/representation-cache/+merge/25895 | 15:22 |
| intellectronica | leonardr: cool. let me just finish a smaller branch i've started and i'll do yours | 15:22 |
| leonardr | great | 15:22 |
| intellectronica | leonardr: also, can i interested in reviewing a (much smaller) branch of mine? | 15:22 |
| leonardr | intellectronica, sure | 15:23 |
| intellectronica | leonardr: cool, let me just create an mp | 15:23 |
| intellectronica | leonardr: https://code.edge.launchpad.net/~intellectronica/launchpad/expectations-bug-556499-model/+merge/25896) | 15:28 |
| intellectronica | leonardr: very nice improvement. looks good to me. r=me. | 15:43 |
| gary_poster | leonardr: in your branch, when there is no cache, did you contemplate always generating it, even if there are redacted fields? Example: if no cache, generate dict of the entire non-redacted version; else if cache and redacted fields, parse out cache to dict; else return cache. (Now we have a non-redacted dict, if we are still here.) | 15:43 |
| gary_poster | Now, redact dict, turn into JSON, and return. There are variations of that, some of which might be better, but I imagine you get the drift. | 15:43 |
| gary_poster | (looks nice to me too :-) ) | 15:44 |
| === salgado is now known as salgado-lunch | ||
| leonardr | gary: thinking about your comment | 15:55 |
| gary_poster | cool | 15:55 |
| leonardr | gary, i'm not sure what the benefit would be | 16:00 |
| leonardr | also, if there are redacted fields we _cannot_ calculate an unredacted cache due to the security policy | 16:01 |
| gary_poster | leonardr: the goal would be to create a source for further cache hits. This could be particularly important for objects that frequently have one or more fields redacted. In that case, the cache would rarely or, in the worst case, never be filled (and therefore never or rarely used). Since DB access is the main expense, you discovered, I strongly suspect that loading JSON and redacting will be significantly cheaper tha | 16:05 |
| gary_poster | creating the JSON. | 16:05 |
| gary_poster | Also, I'm skeptical of "cannot"; isn't it just a matter of doing the usual work with an unproxied object? | 16:05 |
| leonardr | yes, we would have to strip the proxy | 16:07 |
| leonardr | ok, i see what you're saying. we would cache it all the time, whether we were sending a redacted version or not | 16:10 |
| gary_poster | right | 16:10 |
| leonardr | i could certainly do that in a future branch. do you know of launchpad objects that typically have redacted fields? | 16:11 |
| gary_poster | bac would probably know, but he's out. My first guess: anything private, or (perhaps more interesting, perhaps not) anything referring to something provate. | 16:13 |
| gary_poster | private | 16:13 |
| leonardr | if an object's url contains private information, a link to that url would be redacted | 16:14 |
| gary_poster | so, that's an example? | 16:14 |
| leonardr | but i don't know of any specific launchpad object that does that. it's something to look for | 16:14 |
| gary_poster | bugs that are marked as security issues | 16:15 |
| gary_poster | private projects | 16:15 |
| gary_poster | private teams | 16:15 |
| gary_poster | private bugs | 16:15 |
| leonardr | so anything that links to those objects might end up redacted | 16:15 |
| gary_poster | (and there's more coming, if I understand correctly) | 16:16 |
| gary_poster | right | 16:16 |
| leonardr | ok, let's get the basic cache working, make sure it improves performance in real situations, and then i'll work on that | 16:17 |
| gary_poster | cool, makes sense | 16:17 |
| adiroiban | danilos: I have added a comment for bug 583979 . Maybe we can have the pre-implementation chat now. What do say ? Do you have time? | 16:21 |
| leonardr | intellectronica: r=me, sorry for the delay | 16:44 |
| intellectronica | leonardr: thanks! | 16:44 |
| === matsubara is now known as matsubara-lunch | ||
| === jtv1 is now known as jtv | ||
| rockstar | intellectronica, are you still reviewing? | 17:18 |
| intellectronica | rockstar: yeah. got my review? | 17:18 |
| intellectronica | any questions? | 17:18 |
| rockstar | intellectronica, do I owe you a review? | 17:19 |
| intellectronica | i can't start on a new branch, though | 17:19 |
| intellectronica | rockstar: no, but i reviewed a branch of yours and requested a change | 17:19 |
| rockstar | intellectronica, oh, you did? | 17:19 |
| rockstar | intellectronica, where am I using an inline javascript event handler? | 17:20 |
| intellectronica | rockstar: onclick="return somethingSeomthing..." | 17:20 |
| rockstar | intellectronica, oh, that's an artifact of old code. I just moved it, and I'm going to YUI-ize that soon. | 17:20 |
| === intellectronica changed the topic of #launchpad-reviews to: On Call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| rockstar | intellectronica, I'd move to not make any changes to that in this specific branch, but I'm happy to file a bug about it. | 17:21 |
| intellectronica | rockstar: ok, if you're aware of it and have plans to imrpvoe it later i'm fine with it landing like this | 17:21 |
| rockstar | intellectronica, I have a branch right now that entirely re-does the markup of the branch index page in general. | 17:21 |
| intellectronica | rockstar: cool. make a comment to that effect in the mp and i'll approve it | 17:22 |
| === salgado-lunch is now known as salgado | ||
| * intellectronica sheds a tear as he ends his last ever on call review shift | 17:22 | |
| rockstar | intellectronica, done. | 17:23 |
| rockstar | intellectronica, is this your last week? | 17:23 |
| intellectronica | rockstar: it is | 17:23 |
| rockstar | intellectronica, :'( | 17:29 |
| === deryck is now known as deryck[lunch] | ||
| === gary_poster is now known as gary-lunch | ||
| === matsubara-lunch is now known as matsubara | ||
| === deryck[lunch] is now known as deryck | ||
| === Ursinha is now known as Ursinha-lunch | ||
| === gary-lunch is now known as gary_poster | ||
| === Ursinha-lunch is now known as Ursinha | ||
| === sinzui changed the topic of #launchpad-reviews to: On Call: - || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-afk | ||
| === salgado is now known as salgado-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!