| === gmb changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [adiroiban(bug-532239), gmb(http://bit.ly/dvTzD9)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| rockstar | jml, https://code.edge.launchpad.net/~rockstar/launchpad/create-recipe-error-messages/+merge/30253 | 11:40 |
|---|---|---|
| rockstar | (waiting for the diff right now) | 11:40 |
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [adiroiban(bug-532239), gmb(http://bit.ly/dvTzD9), wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === salgado is now known as salgado-lunch | ||
| === mrevell is now known as mrevell-lunch | ||
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [adiroiban(bug-532239), gmb(http://bit.ly/dvTzD9), wgrant, wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| lifeless | wgrant: details please | 13:09 |
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [adiroiban(bug-532239), gmb(http://bit.ly/dvTzD9), wgrant, wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| wgrant | Gah. | 13:09 |
| lifeless | \o/ | 13:10 |
| lifeless | also | 13:10 |
| lifeless | that queue is terrible | 13:10 |
| lifeless | why doesn't an irc bot do it ? | 13:10 |
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [adiroiban(bug-532239), gmb(http://bit.ly/dvTzD9), wgrant(http://bit.ly/bIztta), wgrant(http://bit.ly/93nHei)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| wgrant | That would be too handy, I guess. | 13:11 |
| === mrevell-lunch is now known as mrevell | ||
| === salgado-lunch is now known as salgado | ||
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [adiroiban(bug-532239), gmb(http://bit.ly/dvTzD9)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === vednis is now known as mars | ||
| === lifeless changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [gmb(http://bit.ly/dvTzD9)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === lifeless changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| wgrant | Argh, I dropped the wrong one. | 14:15 |
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [wgrant(http://bit.ly/bIztta)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| lifeless | why rev 11169 ? | 14:16 |
| wgrant | lifeless: We're not entirely sure. It's potentially useful if OEM has an external archive with binaries of the same version, and they want to override the primary archive's with them. | 14:17 |
| wgrant | I don't really feel like breaking that case -- I presume it was there for a reason. | 14:17 |
| lifeless | is someone checking with OEM that this is the case ? | 14:18 |
| wgrant | I don't know. OEM doesn't exist. | 14:18 |
| lifeless | assumptions like this make me nervous | 14:18 |
| wgrant | Well, assuming the other way might break stuff, so it makes me more nervous. | 14:19 |
| lifeless | sure | 14:19 |
| lifeless | its the unknown I referred to | 14:19 |
| wgrant | I'd like to be sure, but I've no way to be. | 14:20 |
| wgrant | Anyway, I should sleep. | 14:26 |
| wgrant | Thanks for all the reviews. | 14:26 |
| === lifeless changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| lifeless | no probs | 14:26 |
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === jtv1 is now known as jtv | ||
| leonardr | abentley, can i get some eyes on https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/restore-shim-objects/+merge/30292 ? | 16:40 |
| abentley | leonardr, looking | 16:42 |
| === danilos changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [danilo] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| danilos | abentley, one in the queue for you as well :) | 16:43 |
| danilos | MP https://code.edge.launchpad.net/~danilo/launchpad/no-dummy-psl/+merge/30293 | 16:43 |
| abentley | leonardr, your proposal has conflicts. | 16:45 |
| leonardr | argh | 16:45 |
| abentley | leonardr, did you consider using people.get(foo)rather than people(foo) ? Maybe you'd parameterize it as deferrable, but I think it would be less surprising. | 16:51 |
| leonardr | abentley: i don't want to create the possibility of interfering with a named operation called 'get' | 16:52 |
| leonardr | gary: -^ opinion? | 16:52 |
| gary_poster | looking | 16:52 |
| leonardr | abentley, conflict fixed, btw | 16:52 |
| gary_poster | right, abentley, leonardr: this is a "we don't have control of the namespace" problem. That's particularly tricky since this is existing software--if this were an initial release maybe we could have claimed "get" for our own, but as it is that's not what we have available. The only other choice I'm aware of is a mediator of some sort, like a function. I think __call__ is the best available compromise, myse | 16:54 |
| leonardr | gary: actually we have (informally) reserved the lp_* namespace | 16:55 |
| gary_poster | hm | 16:55 |
| leonardr | but i think () is better than .lp_get() | 16:55 |
| gary_poster | yeah, I prefer it | 16:55 |
| gary_poster | but good to know about lp_ | 16:55 |
| gary_poster | Maybe we should make that more formal at some point | 16:56 |
| gary_poster | or enforced | 16:56 |
| leonardr | gary: in lazr.restful? prohibit named operations called lp_* | 16:57 |
| gary_poster | right | 16:57 |
| gary_poster | kind of odd for lazr.restful's namespace to be lp_ but I've seen odder things :-) | 16:57 |
| lifeless | surely lazr.restful is meant to not enforce launchpadlib special things? | 16:58 |
| lifeless | perhaps a policy mechanism launchpadlib can configure? | 16:58 |
| gary_poster | isn't that too late? | 16:58 |
| abentley | lifeless, they aren't launchpadlib-specific, AIUI. | 16:58 |
| gary_poster | correct | 16:58 |
| lifeless | abentley: lp_* being blacklisted would be launchpad specific though, wouldn't it ? | 16:59 |
| gary_poster | np | 16:59 |
| gary_poster | no | 16:59 |
| gary_poster | I mean | 16:59 |
| gary_poster | IOW | 16:59 |
| * lifeless pays attention | 16:59 | |
| abentley | lifeless, no, because lp_* contains things that aren't launchpad-specific. | 16:59 |
| abentley | lifeless, AFAIK, lp_* contains only things that are lazr.restful-specific. | 17:00 |
| gary_poster | it would be nicer if the prefix were lr_ (lazr.restful) or some other generic prefix, but the intent is that lp_* be a namespace for lazr.restfulclient (or things that depend on it like launchpadlib) could claim | 17:00 |
| gary_poster | without fear of named operations stomping on them | 17:00 |
| leonardr | the name was chosen before we decided to split the web service code into lazr.restful | 17:01 |
| lifeless | ok | 17:01 |
| lifeless | the lp mislead me, I get it now. | 17:01 |
| gary_poster | understood | 17:01 |
| lifeless | And its presumably too late to change. | 17:01 |
| leonardr | right | 17:01 |
| lifeless | a few randoms | 17:01 |
| lifeless | you could add a _lr_* prefix as well | 17:01 |
| gary_poster | well, "too late" is subjective | 17:01 |
| lifeless | and encourage new special things to go there. | 17:01 |
| lifeless | (or lr_*, or whatever makes sense - I don't know this part of the stack well enough to have any real idea of the issues we're dealing with yet) | 17:02 |
| gary_poster | but yes, my position is coming from the perspective that it is too late | 17:02 |
| gary_poster | +.5 from me | 17:02 |
| abentley | leonardr, have you considered providing exception_for rather than error_for, so that you don't have to special-case 304? | 17:03 |
| leonardr | abentley: i don't follow (but i do know what part of the code you're talking about). how would that help? what do you envision exception_for doing? | 17:05 |
| gary_poster | leonardr: you asked me to look at https://code.edge.launchpad.net/~leonardr/launchpadlib/improve-workflow/+merge/29849 since poolie is unavailable. It looks like you still need to respond to his last comment, which seems reasonable to me. | 17:06 |
| gary_poster | He also asks about load, and "Can we find out from the logs how many times this is called at the moment?" Do I understand correctly that the current answer is "0, because we haven't deployed it yet"? Our logs will track this; we will have visibility on it already if it is one of our top 200 URLs | 17:06 |
| abentley | leonardr, exception_for would return an exception that is not necessarily an error. e.g. for 304, it could return HTTPError(response, content) | 17:06 |
| leonardr | abentley: so it would call error_for and create an HTTPError if error_for returned none? | 17:08 |
| leonardr | that wouldn't get rid of the special case | 17:08 |
| leonardr | gary: i think poolie responded to that review after i asked you to look at it? taking another look | 17:08 |
| gary_poster | k | 17:08 |
| abentley | leonardr, you have a comment explaining why you're not invoking error_for. It would allow you to use exception_for everywhere, and not have to explain anything. | 17:10 |
| leonardr | abentley: in other words, i could change what error_for does and rename it? | 17:11 |
| abentley | abentley, yes. | 17:11 |
| abentley | leonardr, yes. | 17:11 |
| leonardr | abentley, the problem with that is i'd have to move out the code that decides which response codes are exception-worthy (since they woudl all be exception-worthy) | 17:11 |
| leonardr | which, again, it's just moving code around, but i'd be moving normal-case code out of a function so i could move special-case code out of a special case | 17:12 |
| danilos | abentley, hey, do you think you'd have time to look at https://code.edge.launchpad.net/~danilo/launchpad/no-dummy-psl/+merge/30293 (it's mostly code simplification) | 17:18 |
| === flacoste is now known as flacoste_lunch | ||
| abentley | danilos, I expect so. | 17:25 |
| leonardr | gary: we can measure current POST requests to +access_token | 17:26 |
| leonardr | with the new launchpadlib those requests will be approximately multiplied * number of seconds people spend waiting for their browser to load, reading instructions, etc | 17:27 |
| gary_poster | leonardr: yes, but we don't separate them from GET requests; is that important? | 17:27 |
| gary_poster | looking to see if it is in to 200 list... | 17:27 |
| leonardr | gary: i don't think there will ever be a GET request to +access_token | 17:28 |
| gary_poster | k | 17:28 |
| gary_poster | leonardr: it is not one of our top 200 URLs by total hits (https://devpad.canonical.com/~stub/ppr/lpnet/latest-daily-top200.html). Therefore we don't have easy access to it. We could run a report manually, perhaps, or we could change the "categories" section of the performance report to match just that page | 17:30 |
| leonardr | gary: can we grep the raw logs? | 17:30 |
| gary_poster | sure | 17:30 |
| leonardr | i'll do that | 17:30 |
| gary_poster | ok | 17:31 |
| danilos | abentley, ok, I am leaving now so you don't have to do it if you want to do it on-call :) cheers | 17:33 |
| danilos | abentley, I won't mind if I find a review in the morning though ;) | 17:33 |
| abentley | danilos, I'll see how it goes, then. | 17:34 |
| danilos | abentley, sure, ta | 17:34 |
| === danilos is now known as daniloff | ||
| leonardr | gary: | 17:38 |
| abentley | leonardr, having looked at the code, I'd probably handle the 304 + content != '' handling first, then special-case ('If-None-Match' | 'If-Modified-Since') + 304, and otherwise let exception_for decide whether to raise. | 17:38 |
| leonardr | $ grep access-token launchpad-access*.log-20100717 | wc -l | 17:38 |
| leonardr | 7 | 17:38 |
| gary_poster | that's one machine I assume, leonardr, but still, yeah, that seems pretty indicative that this shouldn't be a problem for now | 17:39 |
| abentley | leonardr, that's just a counterexample, not request. | 17:40 |
| leonardr | abentely, i'll take a look | 17:40 |
| leonardr | gary: | 17:41 |
| leonardr | $ find ./ -maxdepth 2 -name launchpad-access*.log-20100717 | xargs grep access-token | wc -l | 17:41 |
| leonardr | 12 | 17:41 |
| gary_poster | leonardr: cool | 17:45 |
| leonardr | gary: i can also make the poll time a parameter so we can easily release a changed version if it proves a problm | 17:48 |
| gary_poster | leonardr: ...I guess. Would the parameter really make the change noticeably easier? I mean, this does seem like the sort of thing that ought to be parametrized, so it sounds good generically, but more from a theoretical rather than a practical perspective. | 17:49 |
| leonardr | gary: yeah, just trying to anticipate trouble | 17:50 |
| gary_poster | understood | 17:50 |
| === flacoste_lunch is now known as flacoste | ||
| deryck | abentley, hi. I'd like to get in the queue for review please. For the code portion of a UI branch. | 18:47 |
| === deryck changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [danilo, deryck] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: deryck || queue: [danilo] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| abentley | deryck, why do we want to move dupes across when making a bug a dupe that itself has duplicates? | 19:31 |
| leonardr | gary, fyi, i posted a response to the improve-workflow branch | 19:33 |
| gary_poster | leonardr: oh ok | 19:33 |
| deryck | abentley, on call, sorry. short story -- lot of manual work to move them. | 19:35 |
| abentley | deryck, it just seems like it would be simpler to just show them as dupes via the dupe, and that way you'd be able to undo the operation. | 19:36 |
| deryck | abentley, hmmm, I see your point. However, this part of the change was actually reviewed earlier, i.e. the ability to move dupes. And is something people have wanted for awhile... | 20:00 |
| deryck | abentley, this current branch just makes the UI aware of the movement. | 20:00 |
| abentley | deryck, okay, well the code itself looks fine. | 20:05 |
| abentley | deryck, btw, it would help to know the title of the bug you're becoming a dupe of. Is that shown when marking dupes? | 20:06 |
| deryck | abentley, no. Because we would have to do an XHR request and wait on the response to open the widget. And that delay didn't seem worth the title at this point... | 20:07 |
| deryck | abentley, eventually, we'll actually do the dupe finder from +filebug in widget, which would provide all that. | 20:08 |
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === abentley changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!