| mwhudson | thumper: want to review this code import branch? | 02:53 |
|---|---|---|
| thumper | mwhudson: ok | 02:53 |
| mwhudson | thumper: https://code.edge.launchpad.net/~mwhudson/launchpad/code-import-optimizations/+merge/23206 | 02:54 |
| mwhudson | thumper: the stacking idea didn't work | 02:54 |
| thumper | ok | 02:55 |
| thumper | mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/fix-schema/+merge/23209 | 05:49 |
| thumper | I have another too that I've been meaning to fix for ages: | 05:49 |
| thumper | https://code.edge.launchpad.net/~thumper/launchpad/scanner-awesomeness/+merge/23211 | 05:49 |
| mwhudson | thumper: "ackward" is a typo | 06:02 |
| mwhudson | (second one) | 06:02 |
| thumper | :) | 06:02 |
| mwhudson | thumper: also assertFoundRevisionNumber and test_not_found are separated by two lines, should be one | 06:03 |
| mwhudson | + self.assertIs(None, iterator.iterator) | 06:04 |
| mwhudson | seems to be testing an implementation detail really | 06:04 |
| thumper | it is | 06:04 |
| thumper | but I used to to confirm that it was broken | 06:04 |
| thumper | not working as intended | 06:04 |
| thumper | working, but weirdly | 06:04 |
| thumper | I can remove it | 06:05 |
| mwhudson | thumper: i think it would be slightly better | 06:05 |
| mwhudson | (the test principle is "go in by the front door" or something) | 06:05 |
| thumper | better to remove it? | 06:06 |
| mwhudson | please | 06:10 |
| mwhudson | thumper: some other small comments on the merge proposal | 06:10 |
| thumper | mwhudson: thanks | 06:10 |
| thumper | W | 06:18 |
| thumper | T | 06:18 |
| thumper | F | 06:18 |
| thumper | :( | 06:18 |
| thumper | ✁☹ | 06:20 |
| thumper | mwhudson: parallel iteration fails | 06:32 |
| thumper | mwhudson: should I just document and ignore? | 06:32 |
| thumper | mwhudson: or I could make it more like the recipe at http://code.activestate.com/recipes/576941-caching-iterable-wrapper/ | 06:36 |
| mwhudson | thumper: hmm | 06:37 |
| mwhudson | thumper: does it fail noisily? | 06:37 |
| thumper | mwhudson: no just very incorrectly | 06:37 |
| mwhudson | thumper: :( | 06:37 |
| mwhudson | oh i guess self.data is shared between the iterators | 06:38 |
| thumper | no I understand the activestate recipe more | 06:38 |
| thumper | it is licenced under MIT | 06:38 |
| thumper | does it really matter? | 06:38 |
| mwhudson | i'm a bit torn | 06:39 |
| mwhudson | because clearly for your usecase it doesn't matter, no | 06:39 |
| mwhudson | but leaving bombs in the source like this to unexpectedly blow up at people doesn't sound good | 06:39 |
| thumper | make it similar? | 06:40 |
| mwhudson | thumper: i think that activestate recipe is still a bit more complicated than you need | 06:40 |
| mwhudson | (e.g. pickling support) | 06:40 |
| thumper | yeah, I didn't care about that | 06:40 |
| thumper | nor the __new__ optimisation | 06:40 |
| thumper | but the indexing is useful | 06:41 |
| mwhudson | well, the indexing stuff is what makes it work in parallel isn't it? | 06:41 |
| thumper | uh ha | 06:42 |
| mwhudson | um, maybe not | 06:42 |
| thumper | actually we can't make it threadsafe | 06:45 |
| thumper | but that shouldn't matter | 06:45 |
| mwhudson | that's ok i think | 06:45 |
| mwhudson | because if you're using threads, you should expect your program to be confusing anyway | 06:46 |
| * mwhudson runs away | 06:46 | |
| thumper | heh | 06:46 |
| thumper | ok, I have parallel bits working | 06:46 |
| thumper | mwhudson: http://pastebin.ubuntu.com/412936/ | 06:48 |
| mwhudson | thumper: "# Defer to the generator" <- i think you mean iterable? | 06:49 |
| thumper | mwhudson: yeah | 06:50 |
| mwhudson | thumper: otherwise looks fine | 06:50 |
| jtv | Anybody willing to review a slightly oversized buildfarm branch? | 07:42 |
| jtv | https://code.edge.launchpad.net/~jtv/launchpad/bug-544237/+merge/23214 | 07:42 |
| jtv | morning henninge! | 07:43 |
| henninge | HI jtv! ;) | 07:49 |
| jtv | henninge: looking at your review changes now. Had to get an oversized branch finished first. | 07:51 |
| henninge | jtv: another one? ;-) | 07:51 |
| jtv | Yes, slave build cookies. | 07:51 |
| jtv | henninge: another comma issue (though this time just a suggestion): instead of ":return: the entry or None if processing failed" it would be clearer to say ":return: the entry, or None if processing failed." | 07:53 |
| jtv | (Makes the scoping clearer) | 07:54 |
| jtv | henninge: I see you cut down this assertion: | 07:56 |
| jtv | 53 # Productseries and distroseries will be asserted in getSubset but | 07:56 |
| jtv | 54 # not sourcepackagename. | 07:56 |
| jtv | 55 assert (distroseries is None and sourcepackagename is None or | 07:56 |
| jtv | 56 distroseries is not None and sourcepackagename is not None), ( | 07:56 |
| jtv | 57 "Please specify distroseries and sourcepackagename together.") | 07:56 |
| henninge | yes ... ? | 07:57 |
| jtv | The comment is useful for reviewers, but I wouldn't bother with it in real-world code. May I suggest simplifying the condition to "(distroseries is None) == (sourcepackagename is None)"? | 07:57 |
| jtv | BTW lots of good changes here, thanks. | 07:57 |
| henninge | jtv: I am not sure that "Noneness" concept is clearer to read. | 07:58 |
| jtv | You could even just say "distroseries is None or sourcepackagename is not None" | 07:58 |
| henninge | that I like ;) | 07:58 |
| jtv | It's not quite as tight, but I don't think the case that it ignores matters much. | 07:59 |
| henninge | hm, which one is that? | 07:59 |
| jtv | Where the caller specifies a sourcepackagename but not a distroseries. | 08:00 |
| jtv | (Sounds bad, but combined with the checks in the utility... anyway, these are assertions, not supposed to trigger at all) | 08:00 |
| henninge | well, in that case the assertion in SubSet will trigger. | 08:01 |
| jtv | Right, combined with the checks in the utility it doesn't matter. | 08:02 |
| jtv | Also in the approver, "Because a generic path (i.e. messages.pot) does not provide [...]"—there are other generic paths besides messages.pot, right? So use "e.g." not "i.e." | 08:02 |
| jtv | henninge: I'm off for a quick bite. See comment above. Also, could you give len_templateset a name that can stand on its own? | 08:29 |
| === jtv is now known as jtv-eat | ||
| === jtv-eat is now known as jtv | ||
| jtv | henninge_: did you see my notes about "i.e." vs. "e.g." and about len_potemplateset? | 09:56 |
| === henninge_ is now known as henninge | ||
| henninge | jtv: yes, all incorporated | 09:57 |
| jtv | henninge: great, thanks | 09:57 |
| henninge | Notes Inc. | 09:57 |
| henninge | ;-) | 09:57 |
| jtv | heh | 09:59 |
| jtv | henninge: I'm going through your changes one commit at a time, so it's possible that I'll see some things that you've already changed. | 10:00 |
| jtv | wgrant: the big problem now is to get a reviewer for the cookie branch. | 10:05 |
| noodles775 | intellectronica: are you around for reviews today? | 10:06 |
| wgrant | jtv: :( | 10:06 |
| wgrant | It's only a little oversized. | 10:06 |
| wgrant | And then only be s/ID/Cookie/.. | 10:06 |
| wgrant | s/be/by/ | 10:06 |
| jtv | wgrant: yes, but the people I've approached so far wouldn't have had time for 700 lines either. | 10:07 |
| noodles775 | jtv: put it in the queue now :) | 10:07 |
| intellectronica | noodles775: sure | 10:07 |
| === 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 | ||
| jtv | intellectronica: got one for ya! | 10:08 |
| intellectronica | noodles775: which branch? | 10:08 |
| jtv | It's slightly oversized though: https://code.edge.launchpad.net/~jtv/launchpad/bug-544237/+merge/23214 | 10:08 |
| noodles775 | intellectronica: I was actually trying to get jtv to ask you to review his branch :) | 10:08 |
| jtv | (Then again, 888 is a very lucky number according to the Chinese) | 10:08 |
| noodles775 | intellectronica: thanks. | 10:08 |
| jtv | for which, noodles775, thank you. :) | 10:08 |
| intellectronica | jtv, noodles775: np | 10:08 |
| jtv | \o/ | 10:08 |
| jtv | thanks | 10:08 |
| === intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| intellectronica | jtv: r=me | 10:53 |
| === 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 | ||
| jtv | intellectronica: wonderful, thanks! | 10:54 |
| jtv | henninge: done with your branch. Phew! | 11:56 |
| henninge | jtv: great, thanks! | 11:56 |
| === stub1 is now known as stub | ||
| === intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: {lunch} || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === mrevell is now known as mrevell-lunch | ||
| === Ursinha_ is now known as Ursinha | ||
| === matsubara-afk is now known as matsubara | ||
| === 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. can you please send this branch to ec2-test https://code.edge.launchpad.net/~adiroiban/launchpad/bug-548999/+merge/22271? thanks! | 13:15 |
| intellectronica | adiroiban: sure | 13:15 |
| === mrevell-lunch is now known as mrevell | ||
| jelmer | t/opic | 14:22 |
| jelmer | argh | 14:22 |
| === abentley changed the topic of #launchpad-reviews to: On call: intellectronica, abentley || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === barry` is now known as barry_ | ||
| === barry_ is now known as barry | ||
| deryck | intellectronica, abentley -- I have a branch for review when one of you has availability. | 15:23 |
| === deryck changed the topic of #launchpad-reviews to: On call: intellectronica, abentley || reviewing: -, - || queue: [deryck] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| intellectronica | deryck: i can review | 15:24 |
| === intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica, abentley || reviewing: deryck, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| deryck | intellectronica, great, thanks! https://code.edge.launchpad.net/~deryck/launchpad/enable-test-inline-subscriber/+merge/23230 | 15:24 |
| intellectronica | deryck: r=me | 15:27 |
| === intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica, abentley || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| deryck | intellectronica, great, thanks! | 15:27 |
| leonardr | intellectronica or abentley, please review https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/user-agent/+merge/23237 | 16:15 |
| intellectronica | leonardr: on the phone. will have a look later if abentley doesn't pick it up | 16:15 |
| leonardr | all right | 16:15 |
| abentley | leonardr, looking. | 16:22 |
| abentley | leonardr, have you considered using standard http header parameters instead of your own syntax? | 16:28 |
| leonardr | abentley: you mean like matrix parameters? or is there something specific for headers? | 16:28 |
| leonardr | oh, i bet you're talking about the parameters that go eg. with an uploaded file | 16:29 |
| leonardr | afaik that's only used in Content-Disposition, but i could be wrong and that might be more readable | 16:30 |
| abentley | I mean like encoding in "Content-type: text/plain; encoding=utf-8" | 16:31 |
| leonardr | ok, yes, it's in a number of places | 16:31 |
| abentley | leonardr, (actually, a correct example is "Content-Type: text/html; charset=ISO-8859-4") | 16:32 |
| === deryck is now known as deryck[lunch] | ||
| abentley | leonardr, yes it's used in a number of places, and IIRC, Python supports querying it. | 16:32 |
| leonardr | abentley: how about this: | 16:33 |
| leonardr | User-Agent: launchpadlib 1.6.1, lazr.restfulclient 1.0.0; oauth_consumer=apport; +cache-control | 16:33 |
| leonardr | or cache-control=true | 16:33 |
| abentley | leonardr, that looks good. I think I prefer cache-control=true. | 16:34 |
| leonardr | ok | 16:34 |
| leonardr | might get lengthy | 16:34 |
| abentley | leonardr, do you actually need cache-control, or could you infer it from the lazr.resfulclient version? | 16:35 |
| leonardr | abentley: i would rather not infer it for two reasons | 16:36 |
| leonardr | 1. i'd rather write server-side code to look for a string rather than parse a version number and compare against some known version number | 16:37 |
| leonardr | 2. other people are writing clients (eg. in java) and i want them to be able to use these features | 16:37 |
| leonardr | i don't want a situation where they pretend to be lazr.restfulclient, the way all browsers pretend to be "Mozilla" | 16:38 |
| abentley | leonardr, it sounded like you were working around a bug in earlier versions of lazr.restfulclient. Am I wrong? | 16:38 |
| leonardr | abentley: i'm working around a current bug in httplib2. so in that sense, old versions of lazr.restfulclient will have a problem and new versions will not | 16:42 |
| leonardr | but the bug is only triggered if the server sends certain information | 16:42 |
| leonardr | so the server needs to distinguish clients that it can send the information to, from clients that will choke on it | 16:42 |
| abentley | leonardr, so it seems unlikely that an arbitrary client (e.g. in Java) would have the same bug. | 16:44 |
| abentley | leonardr, but you're suggesting that all implementations which don't have that bug should assert that they don't. | 16:45 |
| leonardr | abentley: here's the problem | 16:45 |
| leonardr | old versions of lazr.restfulclient, already installed on thousands of computers, will not send any special user-agent | 16:45 |
| leonardr | the server's response to 'no special user-agent' needs to be 'don't send the data' | 16:46 |
| leonardr | a java client will not have the same bug, but it needs to be able to send a special user-agent to get the special data | 16:46 |
| leonardr | i could send the special data to any user-agent that's not 'python/httplib2' | 16:47 |
| leonardr | but even then, someone else writing a non-lazr.restfulclient httplib2 user agent would like to be able to get the special data | 16:47 |
| leonardr | anyway, i'm willing to get rid of cache-control=true because the bug fix for it coincides with the introduction of a real user-agent system | 16:48 |
| leonardr | but if something similar happens in the future i will insist on something like cache-control=true | 16:49 |
| abentley | leonardr, that seems unfair to pre-existing implementations that don't have the bug. Would disable a feature for all installed lazr.restfulclient versions because a Java implementation's support for it was broken? | 16:50 |
| leonardr | abentley: what specifically seems unfair? | 16:51 |
| leonardr | don't send the data unless a client specifically requests it? | 16:51 |
| leonardr | that's why i proposed some alternate solns | 16:51 |
| abentley | It seems unfair to disable, say, http pipelining to all implementations that have already been using it successfully because it later turns out one implementation is broken. | 16:53 |
| abentley | until the existing implementations update their version strings to say that their implementation isn't broken. Which might be incorrect. | 16:53 |
| leonardr | i don't want to get into the business of parsing version numbers in referer headers, especially of software i don't control | 16:54 |
| abentley | What do you mean by referrer headers? I thought we were talking about user-agents? | 16:55 |
| leonardr | sorry, user-agents | 16:55 |
| leonardr | i was thinking of another time a lot of clients broke because of a change involving Referer | 16:55 |
| leonardr | If version 1.5, and i have no idea when it's going to be fixed, how do i do the restriction? | 16:56 |
| leonardr | (if version 1.5 is broken) | 16:56 |
| abentley | leonardr, so the alternative is disabling features that were previously enabled, until clients are updated to say they support them, right? | 16:56 |
| leonardr | i don't think a feature that was previously enabled would ever become disabled | 16:57 |
| leonardr | cache-control is a new feature that exposed an old bug | 16:57 |
| abentley | leonardr, I see. I didn't get that before. | 16:58 |
| leonardr | so it's more a matter of not giving clients a feature until they prove they're not one of the clients that crashes on it | 16:58 |
| leonardr | anyway, if you want the point is moot because i can simply have the server deny this feature to python/httplib2 clients in general | 17:00 |
| abentley | leonardr, Ah, okay. | 17:00 |
| leonardr | and we can take up a more general solution the next time this happens | 17:00 |
| leonardr | but i'm going to be denying this feature to _all_ python/httplib2 clients, even future ones that have the bug fixed, because i don't control httplib2 and i don't know when the bug will be fixed | 17:01 |
| leonardr | once the bug is fixed i can go back and change that, but that's a pain | 17:01 |
| abentley | leonardr, I think it's the best course though. | 17:02 |
| abentley | leonardr, I agree that it can make sense for clients to specifically request features. I wouldn't want to see the mechanism used to address implementation bugs, though. | 17:03 |
| leonardr | clients generally request features by sending http headers. the only reason to do it in the user-agent instead is if there's an agent-specific problem | 17:05 |
| abentley | leonardr, btw, the python support for header parameter is get_param and set_param, described here: http://docs.python.org/library/email.message.html | 17:06 |
| leonardr | ok | 17:06 |
| abentley | So I'm going to do a needs-fixing review and recommend using standard parameters and just blacklisting python/httplib2 clients for cache-control. Sound reasonable? | 17:09 |
| james_w | leonardr: is this about caching the wadl? | 17:18 |
| leonardr | james_w: yes | 17:18 |
| leonardr | abentley, sure | 17:18 |
| james_w | leonardr: may I have a look at the merge proposal? | 17:20 |
| leonardr | james_w: sure. https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/user-agent/+merge/23237 | 17:20 |
| james_w | please subscribe me to the source branch, that page is private | 17:20 |
| leonardr | it has nothing directly to do with the wadl, it's just creating the ocnditions under which i can land a lazr.restful branch without breaking everything | 17:20 |
| leonardr | well, it shouldn't be | 17:21 |
| leonardr | ok, it's public now | 17:21 |
| === matsubara is now known as matsubara-lunch | ||
| james_w | I'm not sure this is worth the effort of a workaround is it? | 17:23 |
| james_w | I'm not sure what the "special data" is though | 17:24 |
| james_w | I thought it was fixed by removing Cookie from Vary in webservice requests? | 17:24 |
| leonardr | james_w: if there's no workaround, then all existing installations of launchpadlib break when i upgrade lazr.restful | 17:25 |
| leonardr | this is separate from Vary. look at the httplib2 bug | 17:25 |
| james_w | oh, none of this is in the bug report it seems | 17:25 |
| === gary_poster is now known as gary-lunch | ||
| leonardr | james_w: i don't follow. which bug report are you refering to? | 17:26 |
| james_w | the lazr.restfulclient one | 17:27 |
| leonardr | james_w: if it will help i'll file a separate lazr.restfulclient bug saying 'work around httplib2 bug 97' | 17:28 |
| mup | Bug #97: can't mark a bug assigned <Launchpad Bugs:Fix Released by daf> <https://launchpad.net/bugs/97> | 17:28 |
| james_w | leonardr: no, it's ok, I was just working from a stale cache (fnar) | 17:29 |
| leonardr | ok | 17:29 |
| james_w | so you want to introduce cache-control in lazr.restful? | 17:29 |
| leonardr | james_w: yes. and httplib2 gives inconsistent behavior when cache-control is combined with conditional requests | 17:30 |
| leonardr | inconsistent behavior that crashes lazr.restfulclient | 17:30 |
| james_w | right | 17:30 |
| james_w | I can see why you need the workaround now | 17:30 |
| james_w | sorry for the noise | 17:31 |
| leonardr | sure | 17:32 |
| === sinzui changed the topic of #launchpad-reviews to: On call: intellectronica, abentley || reviewing: -, - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | abentley, can you review https://code.edge.launchpad.net/~sinzui/launchpad/series-branch-target/+merge/23056 | 17:56 |
| === intellectronica changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | abentley, This branch is a complete reimplementation of a solution that had a fatal flaw | 17:57 |
| leonardr | abentley, ready for you to take another look | 18:07 |
| === matsubara-lunch is now known as matsubara | ||
| abentley | leonardr, r=me | 18:25 |
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === adiroiban changed the topic of #launchpad-reviews to: On call: abentley || reviewing: sinzui || queue: [adiroiban(bug-561355)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adiroiban | intellectronica: any new about my branch sent to ec2 ? | 18:35 |
| intellectronica | adiroiban: it dies mysteriously and i had to resend it, so it will be another 2-3 hours until it finishes | 18:37 |
| adiroiban | intellectronica: ok. then most probably there are some errors | 18:38 |
| adiroiban | and to see them you will need to keep the session open | 18:38 |
| adiroiban | as no email will be dispached | 18:38 |
| intellectronica | adiroiban: i'll try a run with a terminal open if i don't get a response and will let you know if there are errors | 18:39 |
| adiroiban | thanks! | 18:40 |
| === gary-lunch is now known as gary_poster | ||
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: adiroiban(bug-561355) || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | thanks abentley | 18:51 |
| abentley | sinzui, np. | 18:51 |
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-afk | ||
| mars | abentley, ping, would you be comfortable reviewing https://code.edge.launchpad.net/~mars/launchpad/check-db-revision-target/+merge/23255 ? It is a small Makefile change. | 20:02 |
| mars | you know, I would really like to use Bash colour escape codes in there... | 20:03 |
| abentley | mars, did you consider just doing make schema? | 20:03 |
| abentley | mars, rather than erroring, I mean. | 20:04 |
| mars | abentley, I thought about that, but the problem is that you don't always want 'make schema' stomping on the database. The scenario: | 20:04 |
| mars | I have a devel instance running over here: switch contexts to fix a bug over there. Need to start a dev instance, so I try "make run" in the bugfix branch | 20:05 |
| mars | if "make schema" ran automatically on the mismatch, it would stomp my original dev instance | 20:05 |
| abentley | mars, so what do you do instead of running make schema to do the bugfix? | 20:06 |
| mars | abentley, find the original instance, make sure I can shut it down, do so, then spin up the bugfix instance and db. | 20:07 |
| mars | abentley, ugly, but this is a quick-fix that will save a few annoying minutes of my life. | 20:07 |
| abentley | mars, does the check itself take very long? make run seems to be taking longer and longer... | 20:08 |
| mars | maybe auto-running 'make schema' would work, too, but I did not want to spend too much time on it. | 20:08 |
| mars | abentley, the check takes almost no time, especially compared to other stuff. 'make jsFOO' takes much longer. | 20:08 |
| mars | guess what is next on the hit-list | 20:08 |
| mars | :) | 20:09 |
| abentley | mars, cool. | 20:09 |
| mars | small improvements! One step at a time. | 20:09 |
| abentley | mars, does the exit status propagate to the make exit status? If so, it would be nice to use a unique exit status. | 20:10 |
| mars | abentley, I don't know. | 20:10 |
| abentley | mars, is it deliberate that there's no newline at the end of utilities/check-db-revision.py ? | 20:11 |
| mars | abentley, nope. My editor must not add them | 20:13 |
| abentley | mars, weird. Okay, r=me. | 20:13 |
| mars | abentley, great, thank you! | 20:14 |
| === jtv1 is now known as jtv-afk | ||
| bac | hi abentley, can i get a review? https://code.edge.launchpad.net/~bac/launchpad/bug-559200/+merge/23256 | 20:25 |
| abentley | bac, looking | 20:26 |
| abentley | bac, these doctests look a lot like unit tests. | 20:32 |
| abentley | bac, why is the new transaction.commit needed? | 20:33 |
| abentley | (patch line 128) | 20:33 |
| bac | abentley: without the commit the branch conflict does not occur | 20:34 |
| abentley | bac, r=me | 20:35 |
| bac | abentley: thanks | 20:35 |
| bac | abentley: the testing style is consistent with the rest of the registry browser view tests. each application has settled on a different testing style... | 20:36 |
| abentley | bac, I thought it had been agreed that the doctests should be documentation that had tests. | 20:37 |
| leonardr | abentley: https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/httplib-97-workaround/+merge/23263 , though you might want to wait for my change to the lazr.restful branch | 20:41 |
| abentley | leonardr, are If-Modified-Since and If-None-Match the only ways of getting a 304? | 20:45 |
| abentley | leonardr, it's generally not a good idea to sleep in tests. Can you modify the apparent time seen by CookbookWebServiceClient instead? | 20:48 |
| leonardr | abentley: yes, except for a case with intermediate caches which doesn't apply here | 20:49 |
| leonardr | do you have any suggestions on how to modify the apparent time? | 20:49 |
| leonardr | i don't have direct access to the code that checks the time, it's in httplib2 | 20:49 |
| abentley | leonardr, then I guess it can't be helped. | 20:53 |
| abentley | leonardr, what's the motivation for checking whether the client performed a conditional request? | 20:54 |
| leonardr | abentley, look at the end of the diff | 20:55 |
| leonardr | where it used to make a conditional request and expect an exception | 20:55 |
| leonardr | now it gets a special value and i don't use an exception for a non-exceptional case | 20:55 |
| abentley | I understand that. I just don't understand why you would ever want a 304 to give an exception. Do servers really return it when they shouldn't? | 20:58 |
| leonardr | abentley: no, they don't. that's why if it were to happen it woudl be an exceptional situation | 20:59 |
| leonardr | basically, if for some reason it does happen | 20:59 |
| leonardr | i don't want lazr.restfulclient to say 'oh, the object's new representation is NOT_MODIFIED' | 21:00 |
| leonardr | i want it to crash | 21:00 |
| bac | abentley: it depends on what you call a doctest in registry/docs that are as you'd expect. we also have these view tests in browser/tests that use the doctest framework but are not primarily meant to be documentation. | 21:00 |
| bac | oops, i missed some important punctuation in that first run-on sentence. | 21:00 |
| * bac tries again | 21:01 | |
| bac | abentley: it depends on what you call a doctest. in registry/docs we have doctests that are as you'd expect. | 21:01 |
| abentley | leonardr, that's a very different situation from a 302 or a 500. You're now talking about bugs in the server or a proxy. Still, if you think it's important to do that, you can. | 21:02 |
| leonardr | abentley: i think it's important insofar as i want to preserve the old behavior (raise an exception whenever the response is not 200) | 21:03 |
| leonardr | i don't want to carve out a general exception for 304, just an exception for 304 _when i know how to handle it_ | 21:03 |
| abentley | bac, I don't understand why you would make an exception for certain doctests. Is there something about the doctest framework that makes it superior to unit tests for views? | 21:09 |
| bac | abentley: i'm just trying to explain the history of how we got to where we are. the code team has a strong preference for unit tests. other teams prefer the doctest framework. i think the most important thing to do is to be consistent within the prevailing subculture. | 21:11 |
| bac | abentley: unless or until we make a launchpad-wide decision mandating one way or the other. | 21:12 |
| abentley | bac, I think it's also important to respect the decisions that have already been made. I thought there was consensus that we should only use the doctest framework for testable documentation. | 21:15 |
| abentley | bac, I'll add it to the reviewer meeting agenda. | 21:16 |
| bac | abentley: ok | 21:16 |
| EdwinGrubbs2 | abentley: can you review a fairly simple branch for me? | 21:36 |
| === EdwinGrubbs2 is now known as EdwinGrubbs | ||
| abentley | EdwinGrubbs, sure. | 21:36 |
| EdwinGrubbs | abentley: https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-555734-coc-oops/+merge/23268 | 21:36 |
| abentley | EdwinGrubbs, r=me. | 21:40 |
| EdwinGrubbs | thanks | 21:40 |
| * maxb has 2 trivial branches if anyone's still reviewing - https://code.edge.launchpad.net/~maxb/launchpad/ - but I'm assuming EOD must be nowish for Americas | 22:35 | |
| === matsubara-afk is now known as matsubara | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!