[00:00] if 99.9% requests were really ok though, nagios would be having to making sqillions of requests to trip at all often [00:01] wgrant: what is hard vis-a-vis shared cache? [00:02] lifeless: We could go with a shared SQLite DB for now. But that probably introduces concurrency issues, and is clearly harder than None :) [00:03] It also becomes a lot harder once we de-SPOF guava. [00:03] particularly, using sqlite would mean we'd have to remember it when we load balance across machines [00:03] yes [00:03] Exactly. [00:03] a concern for historydb too [00:03] Does historydb have to be shared? [00:03] I don't really know how it works. [00:03] I heard a rumour that sqlite is webscale? [00:03] yes, although that's suboptimal performance vs not working [00:04] wgrant: sharing is how its faster than the prior cache [00:04] lifeless: not really [00:04] :( [00:04] it's mostly faster than the other cache because it can be updated incrementally [00:05] mwhudson: its faster on additional branches because it can be reused [00:05] the caches are already shared between the two instances of loggerhead running on guava [00:05] yes [00:05] i agree a cache that could be shared across machines would be better [00:23] Do I want to fix ec2 land to stop adding [ui=none]? [00:23] yes [00:25] It's also tempting to fix the formatting of the post-bug-filing alert. [00:30] I think thats tagged high not critical, though I wouldn't obeject [00:31] If it were critical I wouldn't be mentioning it to see if you objected :P [00:34] its really up to curtis to object or not ;) [00:36] StevenK: it is BranchNamespace.renameBranch [00:36] StevenK: it has a param rename_if_necessary [00:46] lifeless: You define the set of critical bugs, and you are around, so you'll do! [00:51] hah [00:52] anyhow, its probably shallow, and would be lovely to have [00:52] and I'm a strong believer in engineer discretion [01:11] StevenK: Hi. [01:21] wgrant: O hai [01:22] StevenK: I have a quick review for you, if you've got a sec. [01:22] https://code.launchpad.net/~wgrant/launchpad/bug-711016-no-ui-clause/+merge/48094 [01:22] wgrant: Did you see my earlier prod? [01:22] [10:11] < StevenK> wgrant: Is http://pastebin.ubuntu.com/560772/ better? [01:24] StevenK: Completely missed that, sorry. [01:24] StevenK: That looks good. Did you see my diff? [01:24] wgrant: Nope, so I went and changed it anyway. [01:25] wgrant: That MP looks good -- with one proviso -- there's still tests that check ui=blah, right? [01:26] StevenK: Yup. [01:27] thumper: https://code.launchpad.net/~wgrant/launchpad/bug-711016-no-ui-clause/+merge/48094 when you can [01:27] StevenK: ack [01:27] StevenK: you have time today to take a look at this (after your other stuff is done) ???? https://code.launchpad.net/~wallyworld/launchpad/recipe-find-related-branches/+merge/47367 [01:29] thumper: what's your verdict about the extra text in the related branches view - as per my comment to your comment :-) [01:32] wallyworld: I don't think the text is accurate, and is likely to be confusing [01:33] thumper: ok. i'll get rid of it [01:40] thumper: I'm preparing a rollback of recipe deletion, are you fine to rs it? [01:40] yep [01:47] You know, it would be really nice if ec2 phoned home. [01:47] So we could have a full branch dashboard. [01:48] Bloody hell, it's not even 1pm and it's already 36degC [01:50] It's already hit the max (38°C) here. [01:50] StevenK: know what you mean. i tempted to go sit in my pool and try not to splash water on the laptop :-) [01:50] Haha [01:51] thumper: Bah! It's *moveBranch* [02:12] lifeless: Thanks. [02:25] huwshimi: Hi. [02:26] wgrant: Hey there [02:27] huwshimi: I'm currently having some fun with the notification CSS. [02:28] wgrant: what kind of fun? [02:28] huwshimi: I want to put some

s in a notification instead of straight text. But then the notification's bottom padding and the

's bottom margin combine to make a huge gap. [02:28] wgrant: and urls plox ? [02:28] wgrant: Ah right [02:29] huwshimi: I can fix it by adding a .informational > p:last-child {margin-bottom: 0;}, but that seems bad. [02:29] https://code.launchpad.net/~wgrant/launchpad/bug-654585-linkify-bug-acknowledgement is the branch. [02:31] wgrant: so, using last-child is a good thing, but it only works in modern browsers. The usual way around it is to apply a class of "last" to the last

. [02:32] wgrant: This is also kind of ugly, but it's how we've been solving the the problem for years. [02:32] huwshimi: Even IE7 supports it, right? [02:33] We already use last-child somewhere... possibly breadcrumbs. [02:33] wgrant: Actually I believe it only works in IE9. But I could be wrong about that [02:34] You're right. [02:34] WTF [02:34] wgrant: Indeed. [02:37] Do we care enough about IE<=8 to fix it there too? [02:40] Oh, IE>=7 support :first-child, but not :last-child. Lovely. [02:43] sweet [02:43] === Top 10 Time Out Counts by Page ID === [02:43] Hard / Soft Page ID [02:43] 644 / 8052 Archive:+index [02:44] wgrant: did I just paste three timeouts ? [02:45] lifeless: Just the one. [02:45] But three lines. [02:45] 15:43 < lifeless> 644 / 8052 Archive:+index [02:45] 15:43 < lifeless> 200 / 410 BugTask:+index [02:45] 15:43 < lifeless> 91 / 194 Distribution:+bugs [02:46] Also I hate the latest freenode rev [02:46] Only got the first of those. [02:46] *hate* [02:46] wgrant: I'm not sure. So I think we should take a time + effort vs how badly it breaks attitude. [02:46] 68 / 33 Branch:+index [02:46] 43 / 14 Archive:+copy-packages [02:46] 32 / 295 POFile:+translate [02:46] 24 / 283 Distribution:+bugtarget-portlet-bugfilters-stats [02:46] 16 / 49 Archive:+packages [02:46] 11 / 40 Product:+code-index [02:46] 10 / 16 DistroSeriesLanguage:+index [02:47] huwshimi: Mm. I suppose I could hack text_to_html to add a class to the last paragraph. [02:47] But I hate IE. [02:47] wgrant: in this case, if the break is aweful and it is easy enough to fix, then do it. [02:48] wgrant: I believe we have low numbers of IE users, but enough to be significant. [02:50] * thumper heads to coffee [02:51] Do we have accessible stats of browser usage? [02:51] Some people have Google Analytics access. [02:51] sinzui was looking at them during the Thunderdome. [02:51] wgrant: I personally have the opinion that it is time to let things break on IE6, because as long as we attempt to make it look half decent, organisations etc. will not bother to upgrade, but once the web experience starts to break then the pressure from users will be enough to make organisations make the upgrade [02:52] huwshimi: Certainly, disregarding IE6 has been good standard policy for a while. [02:52] huwshimi: a massive hack attack on it wasn't enough ? [02:52] lifeless: No. [02:53] lifeless: You would think so [02:54] infact I think that even if a couple of major websites were broken on IE the pressure would mount (e.g. facebook) [02:55] but those sites have the most to lose ;) [02:57] lifeless: Well facebook is phasing out ie6 support for some of their features already [02:58] \o/ [02:59] Surely businesses will play the "You shouldn't be viewing Facebook at work anyway" card, and not upgrade anyway? :-) [03:01] Oh good, just to make it easy the thing that text_to_html iterates over is a generator. [03:03] wgrant: Is this so that you can add the last class? [03:03] huwshimi: Yes. [03:03] wgrant: I'm guessing that means it's going to be hard to add the class? Or impossible? [03:04] huwshimi: Well, I can turn the generator into a function that returns a list instead. [03:11] Ugh. [03:12] * wgrant gives up for a while and lunches. [03:18] wgrant: When you get back let me know and we can talk about this again. [03:18] thumper: So I'm struggling with this rename-recipes-when-merging branch -- do you think I should subclass IBranchNamespace and then call it from the merging code? [03:22] if i were to raise a bug to address a performance issue, that would be a Defect rather than an Improvement? Or not? [03:23] wgrant: we don't really have defect/improvement in our workflow [03:23] bah [03:23] wallyworld: ^ [03:24] sooooo, the verdict? [03:24] wallyworld: So I don't understand the question [03:24] i'm talkin abou the kanban board [03:24] sorry, not enough context [03:24] i knew what i meant :-) [03:26] wallyworld: so, you're on the recipe squad right? [03:26] wallyworld: in principle everything you're working on is improvement [03:26] yeah, but it's to fix the +daily-builds performance issue [03:27] ok, i'll use improvement, thanks [03:27] wallyworld: its an improvement :) [03:28] lifeless: many times i'm included to consider performance issues as defects :-) [03:28] another question too: i've had an ec2 failure email, with no errors attached but instead the message "SMTPError: SMTP error: 552 Message size exceeds maximum permitted" [03:28] where do i look to see what happened? [03:30] blah [03:30] the compressed subunit log was larger than the mailserver would accept [03:31] 'someone' should make ec2 test attach that to the branch using the subunit API in launchpad [03:31] thumper: you've added brand new things to the API before, right ? [03:32] thumper: new types etc, I mean. [03:33] lifeless: in the meantime, how do i get the test log? i guess it's lost when the ec2 instance shuts down? [03:33] stub: what's the table/query to see what DB patch a given DB is currently up to? [03:33] wallyworld: yeah :( [03:33] bollocks :-( [03:34] select * from launchpaddatabaserevision order by major,minor,patch; [03:34] ta [03:35] lifeless: it was the recently done sql bind parameters branch. perhaps something failed so spectacularly that the log file got too big? [03:35] spm: I'm guessing that means we has tree ? [03:35] wallyworld: thats quite possible [03:35] lifeless: no not yet. I was wonder htf qastaging get's it's db patches at all. [03:36] spm: it doesn't [03:36] spm: it gets restored to. [03:36] spm: it *must not* apply db patches because its what we QA our deployments on [03:37] yah. and hence was wondering how it managed to maintain some semblance of code/db sync at all. [03:38] spm: its meant to be restored weekly with staging, and for db deploys it has the patches applied to it. [03:38] however something seems to be quirky there [03:38] yes. hence my questions and htf'ing [03:39] spm: not functionally quirky, if that makes sense. [03:39] :-) [03:40] wgrant: Do you think I should just force a build to get buildbot/pqm happy again? [03:54] wallyworld: hi [03:55] lifeless: hello [03:55] wallyworld: we have db transactions, reading twice from the DB is pointless. [03:55] wallyworld: perhaps I'm misunderstanding something. [03:55] you mean in the deleted recipe branch? [03:55] yes [03:56] unless you're in two separate transactions (which you may need to be to avoid having long running transactions on big uploads) [03:56] i wasn't sure of the transaction boundaries and so was being (overly) cautious [03:56] if you're in two separate transactions, then I'd just check at the end [03:56] just to avoid any possible race conidions [03:56] wallyworld: well, it raises red flags to me [03:56] i didn't want to go to the expense of trying a build if there was no pint, hence the check up front [03:56] wallyworld: thusly: it makes it more complex, and its either not needed, or it is needed but wasn't clear that it is. [03:57] wallyworld: I guess I'm asking for more surety that it is/isn't rather than a hopeful statement that doesn't fit with transaction semantics. [03:58] wallyworld: separately, whats this daily build performance bug you're going to look at? I'm curious. [03:59] not so much transaction semantics but where the transaction boundaries are. i'll revisit it though.... [03:59] https://devpad.canonical.com/~lpqateam/ppr/lpnet/latest-daily-timeout-candidates.html [03:59] ah, a page dear to my heart [03:59] +daily-builds, about 1/2 way down [03:59] ah yes [03:59] I was mentioning this to tim the other day [04:00] never completes in < 5 seconds [04:00] suggests its doing -way- to much work [04:00] yeah :-( [04:01] to me that's a defect but..... [04:01] ... i've added an improvement card [04:01] :) [04:06] spm: can you toss a haproxy status page screeny my way? We're seeing more might-be-GIL stuff, and as we haven't done single-threaded appservers yet, I can't eliminate it... [04:06] but the queue dpeth stuff often gives decent hints [04:06] prod? [04:07] yeah, servers C,E,H [04:11] StevenK: Always. [04:11] hmm is +code-index memcached? [04:11] Fricken buildbot [04:13] ok, phew, new bugs for new timeout limit filed [04:29] wallyworld: https://bugs.launchpad.net/launchpad/+bug/711072 - I've doctored it up to add more info for you [04:29] <_mup_> Bug #711072: RootObject:+daily-builds times out < https://launchpad.net/bugs/711072 > [04:29] wallyworld: I hope that helps [04:31] lifeless: thanks, although you've taken all the joy away of me finding that myself :-) i don't mind getting down and dirty with a profiler you know :-) [04:34] wallyworld: I logged into devpad [04:34] cd /srv/launchpad.net-logs [04:34] grep +daily-builds lpnet/*/2011-02-01 -r [04:35] (I didn't get the joy of profiling either) [04:35] lifeless: thanks for the tip [04:52] buildbot, DIAF. Whoever thought the xmlrpc interface to it is documented is brain-dead [04:52] StevenK: it was fairly straightforward when I read the code [04:53] I love how buildbot-poller hard codes lucid_lp and lucid_db_lp in multiple places. [04:59] lifeless: yes in the past I have, but not for ages [04:59] * thumper is making dinner [05:11] wgrant: where are you up to on archive:+index ? [05:15] lifeless: The OOPSes I've looked a recently have been for a copy archive, and resulted in bug #709717. [05:15] <_mup_> Bug #709717: ArchiveView.num_pkgs_building can be very slow < https://launchpad.net/bugs/709717 > [05:15] I haven't checked the general case since Julian's fixes. [05:16] wgrant: can you pleae put page ids in titles of such bugs? helps me at least ;) [05:17] lifeless: Sure, sorry. [05:17] no need to apologise [05:17] its informal [06:13] Have a good rest of the day people. [06:26] StevenK: so whats up with Revision 12277 can not be deployed: needstesting [06:30] wgrant: still around ? [06:30] wgrant: why in bug 691478 do we need a union ? [06:30] <_mup_> Bug #691478: Archive:+index timeouts < https://launchpad.net/bugs/691478 > [06:34] lifeless: There are a couple of places we use them. [06:34] One is to determine the set of builds that we need to consider: we grab those in the archive itself, and union with those that were copied in. [06:38] I'll be glad when wallyworld's parameter saving patch gets stashed [06:38] lol [06:43] ok, I think I need to call it a day [07:21] lifeless: Sorry, I was picking up Sarah; still here? [08:03] StevenK: here [08:32] lifeless: I'm rolling it back and not QA'ing since I don't want users to lose information when they merge if they have recipes. [08:36] * wgrant stabs lucid_db_lp. [08:43] StevenK: ok, nowish ? [08:44] lifeless: It has been rolled back, it's tip of devel [08:46] StevenK: sweet [08:56] https://code.launchpad.net/~thumper/lazr-js/multi-line-editor-goodness/+merge/48118 if someone likes looking at styles and javascript [08:56] * thumper is really done [08:56] morning [08:58] * thumper claims 6 bug fixes with that merge proposal \\o/ [08:58] * thumper flees the office [09:00] Low bandwidth good morning. [09:00] coffee shop or 3g? [09:01] good moring [09:03] bigjools: Hahaha. 3G round here? To make a hollow laughing. At the moment I'm doing irc and email over GPRS. Coffee shop will happen later when traffic in town has died away. [09:03] zoiks [09:04] I know. Party like it's 1995. [09:18] Howdy [09:28] open id + redirects = FAIL [09:44] wgrant: around? [09:54] bigjools: Hi. [09:54] Did you break mawson? [09:54] wgrant: no [09:54] :( [09:55] but I am testing on it [09:56] bigjools: What provides bzr lp-land? bzr-pqm? [09:56] wgrant: yes [09:57] wgrant: quick question for your consideration: [09:57] in rescueBuilderIfLost() I want to fail the builder if a nonvirtual builder ends up in ABORTED [09:57] because we can't be sure abort() worked [09:57] what do yo uthink? [09:58] They only end up ABORTED if someone manually kills things. Acting on ABORTING might be more useful. [09:58] wgrant: the manually killing thing is precisely the situation I am considering [09:59] and catches the ABORTING stage too [09:59] see bug 705342 [09:59] <_mup_> Bug #705342: buildlog contains a mix of two different builds < https://launchpad.net/bugs/705342 > [09:59] Hmm, actually, they will also end up ABORTED once the build finishes. [09:59] That's the normal case. [09:59] Sigh. [10:00] hmmmmn? [10:00] there's nothing normal about it [10:00] Builders will eventually recover from ABORTING without manual intervention. [10:00] Because the build will finish. [10:00] I wonder how often that happens. [10:01] well the double build thing happens even when we call cleanSlave after seeing ABORTED [10:01] there's a plethora of slave bugs here [10:01] Right. After manual intervention. [10:01] yes [10:02] (in other news, a few hundred lines of lpland look remarkably identical to autoland) [10:02] I wonder if the same diff applies. [10:02] so if we end up in ABORTED on a nonvirtual slave I am suspicious [10:02] and therefore want to fail it [10:02] because it means the admin has pebkac [10:03] Not always. [10:03] mostly [10:03] I do not know the relative frequencies. [10:03] I suspect that the problematic case is a tiny minority. [10:03] * bigjools greps [10:05] I only see one case [10:07] OK, I should not be able to take a 154 line diff and apply it to a different project with no conflicts. [10:07] I am making the change anyway [10:07] wgrant: :( [10:07] bigjools: OK. [10:07] I guess we'll see how it goes. [10:07] yeah [10:07] if nothing else it'll prevent serious issues [10:08] bigjools: Also, as you may be aware, We have an upcoming Debian release. [10:08] now, to work out why assertFalse doesn't fail for a True value [10:08] yeah [10:08] I've landed the config changes already. [10:08] I saw [10:08] Do we need to do anything beyond creating the new series and adding the series name to iron's crontab? [10:09] not entirely sure tbh, it's been a while since we changed gina config! [10:09] dogfood FTW [10:09] Indeed. [10:10] I know this will work. I was mostly wondering if there were any instructions to create the series, or if we should just imitate the previous one. [10:10] test it on DF [10:10] k [10:11] How do I get the Debian archive onto there? [10:11] Can I sync bits of it from iron or similar? [10:11] let me know if you want to bring it down/up since I am testing [10:11] ummm ask Steve, he did something with it recently [10:11] I can't do it yet, since wheezy doesn't exist. [10:11] Although I guess I could mangle the squeeze indices to look like wheezy. [10:12] * bigjools - > call [10:33] Hi folks how do I make my tests pass in python? I keep saying "pass" but it doesn't work. [11:00] can someone take a look at this and guess as to why, although the assertionerror is raised, the test doesn't fail? http://pastebin.ubuntu.com/560674 === wgrant_ is now known as wgrant [11:16] jml: around? [11:20] Project devel build (407): FAILURE in 5 hr 38 min: https://hudson.wedontsleep.org/job/devel/407/ [11:21] bigjools: am now [11:21] * gmb returns, via Starbucks. [11:21] jml: can you check my paste above please [11:21] The coffee blows, but the wifi is oh so sweet. [11:21] jml: I've got a failure in a twisted test but it's not causing the test to fail [11:22] dunno where the problem is, trying to debug the test runner ... [11:22] gmb: compared to the Dallas hotel coffee, Starbucks was great. :( [11:22] bigjools: yuck. can you paste the test & what happens when you run it? [11:23] Well, yeah. [11:23] (the full class would be nice, actually) [11:23] jml: http://pastebin.ubuntu.com/560899/ [11:24] the if 1==2 is to avoid the condition that makes the test pass [11:28] bigjools: does the breakpoint get triggered? [11:31] jml: yes [11:31] jml: see http://pastebin.ubuntu.com/560674 [11:32] bigjools: the problem is that the assertion error is being swallowed? [11:32] jml: that's what it looks like [11:33] when I debug, I can see it getting recorded as a Failure, then nothing happens when it ends up in fail_if_exception_caught() [11:33] bigjools: OK. Let's try to whip up a minimally reproducing example. [11:33] * bigjools tries something else [11:34] jml: [11:34] def test_failing_test(self): [11:34] self.assertFalse(True) [11:34] does not fail ... [11:35] bigjools: that would be a minimally reproducing example. [11:35] indeed :) [11:35] bigjools: just trying locally, in case it's something in your branch / local config. [11:35] ok [11:35] jml: I added that to TestBuilder FWIW [11:36] cannot import name BuildNavigationMixin... [11:37] circ imports :/ [11:37] I haven't changed anything! [11:37] if you run with -t it doesn't do it [11:37] oh right. [11:37] only if you specify the module [11:37] le sigh [11:38] jml: I've narrowed it down to the presence of this line in the test class: [11:38] run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=10) [11:38] remove that and the simple case fails [11:38] this is rather worrying :)_ [11:39] bigjools: that line says "use the reactor when running tests and wait for deferreds to fire" [11:39] I expect the bug is in ADRT though. [11:40] dammit. I've got to go. [11:40] bigjools: please see if you can reproduce it using just testtools. [11:40] bigjools: file a bug on testtools and I'll fix it when I get back. [11:41] jml: I'll dig [11:41] thanks [12:06] Morning, all. [12:26] jml: https://bugs.launchpad.net/testtools/+bug/711209 [12:26] <_mup_> Bug #711209: When running Twisted tests, failures are not reported < https://launchpad.net/bugs/711209 > [12:34] jelmer: Could you please land that bzr-pqm change? I don't think I have privs. [12:38] wgrant: sure [12:41] wgrant: I really think this kind of stuff should live in another plugin, perhaps the same plugin as lpreview_body [12:41] anyway, not your fault :) [12:43] jelmer: Yes :( [13:10] Hmm, recipes are pretty cool. [13:18] allenap: ping? [13:21] gary_poster: Hi there. [13:23] Hi allenap :-) . Could I have a second review from you on a branch? It is a branch that moves the bug_notification_level from structural subscriptions to the filter. I have one specific question for you, and then I'd appreciate a general review as well. https://code.launchpad.net/~gary/launchpad/move-events-to-filters/+merge/48069 Can you take the time for that? [13:23] it's a big branch :-/ [13:23] gary_poster: Sure :) [13:24] thank you! [13:25] allenap, the specific question is about the delete method on the filter. I pose the question/concern in the second reply to the MP, about the XXX [13:26] gary_poster: Cool. [13:26] thanks again === bac changed the topic of #launchpad-dev to: Performance Tuesday | https://€‹dev.launchpad.net/€‹ | PQM is open | firefighting: - | On call reviewer: - | https://code.launchpad.net/launchpad-project/+activereviews [14:00] abentley, adeuring -- I [14:00] I'm coming :-) [14:05] bac: Um... did you just introduce weird unicode into the topic? :-) [14:24] maxb: no, i tried to remove weird unicode === bac changed the topic of #launchpad-dev to: Performance Tuesday | https://dev.launchpad.net/| PQM is open | firefighting: - | On call reviewer: - | https://code.launchpad.net/launchpad-project/+activereviews === leonardr changed the topic of #launchpad-dev to: Performance Tuesday | https://dev.launchpad.net/| PQM is open | firefighting: - | On call reviewer: leonardr | https://code.launchpad.net/launchpad-project/+activereviews [14:44] bigjools: the test doesn't fail when run purely with testtools [14:44] as in, it fails correctly [14:44] jml: :/ [14:45] did you see the bug? [14:46] bigjools: yeah, I did. [14:46] bigjools: bringing in more of the LP stack now to reproduce [14:47] jml: I have no clue why it thinks that attribute is not settable [14:47] trez bizarre === matsubara is now known as matsubara-lunch [14:48] this would be easier if my laptop wasn't suffering display refresh problems [14:49] * jml tries a different runner [14:50] * bigjools discovers how hard it is to search for an sql statement containing "except" [14:50] hmm. [14:51] so w/ ADRT it passes incorrectly [14:51] w/ SDRT it gives that stack trace that you supplied in the bug [14:51] w/ RT it fails as it ought. [14:51] hummm [14:51] let me eliminate zope.testing [14:51] thanks for debugging [14:52] actually, first, getting rid of LP's base TestCase [14:52] bigjools: np. === salgado is now known as salgado-lunch [14:58] http://paste.ubuntu.com/560985 - huh [14:58] * jml tries with just testtools [15:00] jml: I bet that something is declaring that attribute as a @property when the other classes are not [15:00] easy to grep for it [15:00] which attribute! [15:00] the one in the traceback [15:01] the exception message specifies neither the object nor the attribute name [15:01] I pdb'd far enough to find it [15:01] look in the traceback! last line [15:02] f_locals [15:03] appropriate name [15:03] hmm. [15:03] the other worrying thing is that an except was totally swallowed [15:03] exception [15:04] this is perplexing. [15:05] the exact same test behaves correctly (i.e. fails) when run as part of the testtools suite [15:05] so is the async runner inserting a class into the hierarchy that declares f_locals as a property? [15:06] sorry, not giving this my full attention [15:07] no, it's not. [15:07] if it were, it would fail when run as part of the testtools suite [15:08] hmm. [15:09] bigjools, I seem to be getting None for this when processing a source package recipe build fails. Could some of these relationships be waiting for the upload to succeed before they're set? http://pastebin.ubuntu.com/560988/ [15:09] checking [15:09] bigjools: also, don't worry about attention. it helps me enough to dump new data in the channel :) [15:10] abentley: SourcePackageRelease won't exist will it? [15:10] jml: heh ok :) [15:12] bigjools, I guess not. Is there another way I can find the sourcepackagerecipebuild for a packageupload? [15:12] lemme see [15:12] ok. I have no idea how I got the behaviour I pasted. [15:14] danilos: do you have some time to discuss the remove_translations script (bug 705652)? [15:14] <_mup_> Bug #705652: Make remove_translations side aware. < https://launchpad.net/bugs/705652 > [15:15] adeuring, sure, in say 30 mins, would that work for you? [15:15] danilos: sure. [15:15] thanks, adeuring, and feel free to re-ping me in that time :) [15:16] danilos: ok, will do :) [15:16] abentley: hmmm I don't understand how there is a packageupload at all if the upload failed [15:17] hah [15:17] ok [15:17] at what point are you looking for this data, in the failure code? [15:17] I think it's something imported by test_builder that's playing havoc [15:17] jml: yay.... [15:18] easy enough to figure out [15:18] abentley: if there's a failure, you need to fall back to the data in the changes file really, the database will not be intact and is about to get its txn aborted [15:18] * jml bisects [15:18] two jmls! [15:19] can we put one of you in another timezone? [15:19] bigjools, this is in the archiveuploader, which already uses a PackageUpload object. [15:19] abentley: right, but I doubt it's committed yet [15:19] abentley: what are you fixing/doing exactly? [15:20] bigjools, I am fixing the way the PackageUpload object determines whether it's processing a user upload or a build. [15:20] bigjools, until my changes, it used self.builds for that. [15:20] as in a recipe build [15:21] abentley: the builds won't be signed [15:21] bigjools, as in a recipe build or binary build. [15:21] user uploads are always signed [15:21] abentley: what's the problem you're fixing? [15:22] bigjools, it's sending two emails because part of the code thinks it's doing a user upload and part of the code thinks it's doing a build upload. [15:22] ok [15:22] what are the conditions it's checking? [15:22] and, wtf, why 2 places! [15:25] hah [15:25] ok. [15:25] bigjools, PackageUpload is now using self.builds and getSourceBuild to determine whether it's doing a build. UploadProcessor is using its own, very different self.builds to determine whether it's processing ubilds. [15:25] found the offending line in lp.testing.__init__. [15:26] now have to figure out why it's there before I kill it with great killing. [15:26] s/ubilds/builds/ [15:26] (also, implicit global monkey patches of core libraries should always have a comment) [15:27] jml: and should maybe be avoided, if possible. :-P [15:27] abentley: ok, I'll have a looksee [15:27] jcsackett: well, I guess. [15:28] huh. I reviewed the change. [15:28] jml: i mean, you're talking the realm of tests, where monkeypatching is sort of required. but "implicit global monkeypatch" is sort of a frightening prhase, don't you think? :-P [15:28] abentley: what is calling those, the notify() stuff? [15:28] jcsackett: yeah, it is. [15:28] _Frame.f_locals = property(lambda self: {}) [15:28] wtf! [15:29] jcsackett: here it's really a specific instance of "imports should not have side effects" [15:29] bigjools: looking into it now [15:29] jml: dig. [15:29] bigjools: found the bug and the code review [15:29] 425113 fwiw [15:29] excellent [15:29] bug 425113 [15:29] <_mup_> Bug #425113: Some tests can fail without detection < https://launchpad.net/bugs/425113 > [15:29] bigjools, yes, PackageUpload.notify [15:29] oh the irony [15:29] abentley: right, there's two lots of email that goes out [15:30] so it makes sense [15:30] hmm. [15:30] there was a comment in the MP [15:31] ahh, it got moved, probably by a zealous import formatter. [15:31] bigjools, I think ultimately it would be good to have polymorphism change the behaviour of PackageUpload.notify depending on whether it's a build or user upload. [15:31] abentley: so, the code at the top of notify() is clearly wrong for recipe uploads === almaisan-away is now known as al-maisan [15:32] curse you distribution mirror prober! [15:32] jml: it'll be fun to see how many tests start failing when you remove that [15:33] bigjools, the test for whether it's SECURITY? [15:33] bigjools: thing is, it was added to stop tests from failing silently [15:33] jml: hence: [15:29:34] oh the irony [15:33] yeah [15:33] I guess what I mean is, I don't think anything will fail when I remove it [15:34] abentley: oh actually, scratch that === salgado-lunch is now known as salgado [15:34] oh [15:35] except that maybe some tests are failing silently now [15:35] was a bit too focused to get that [15:35] exactly [15:36] the thing here is that testtools doesn't have the bug that trial has [15:36] abentley: so sending two emails is the right thing in some circumstances [15:36] abentley: do you have a case where that's wrong? [15:36] I'm going to move this monkey-patch into the TwistedLayer, which should do the trick. [15:37] abentley: actually for PPA uploads it should always be one, I am thinking of the distro case [15:37] bigjools, right. [15:37] ffs [15:38] how do I change a bugtask against testtools to be targeted against Launchpad? it says "too many matches" when I type "launchpad" [15:38] jml, use the dropdown thingy. [15:39] abentley: can you point me at the code in the upload processor? [15:39] it's been 2 years since I delved into the notification code [15:40] abentley: ta [15:41] bigjools, lib/lp/archiveuploader/uploadprocessor.py:634 [15:41] abentley: the email bit I mean [15:41] it's not sending email there? [15:42] grar, this code is utterly shite [15:42] bigjools, it's invoking self.build.notify there. [15:42] bigjools, and hey! that's my code! [15:42] lol :) [15:42] bigjools, well, I refactored it, anyhow. [15:43] ok, so that code there is for BFNs [15:43] BFN=Build Farm Notification? [15:43] all the routine notification should be done in PU.notify [15:43] build failure notification [15:44] bigjools, what do we notify about except failure (and success for recipe builds):? [15:44] uploaders always get an email [15:45] for source uploads anyway [15:45] bigjools, and we don't permit binary uploads, right? [15:45] adeuring, heya :) [15:45] abentley: not from user [15:45] s [15:45] hi danilos. shall we talk on mumble? [15:46] adeuring, sure thing [15:46] abentley: process-upload is invoked with --builds in the buildmaster machine and then it knows to allow build uploads [15:46] bigjools, right, that's where UploadProcessor.builds comes from. [15:47] abentley: if you can, I would try and poke all your notification code into PU.notify [15:48] move it away from the build [15:48] then it will be consistent with the existing processing [15:49] and you can easily check to see if it's a recipe upload if you want to tweak any text [15:50] bigjools, I can do that, but PU will still be incorrectly detecting that this is a user upload. === deryck is now known as deryck[lunch] [15:50] abentley: check to see if the upload is signed [15:50] it does that in a couple of places already IIRC [15:51] self.signing_key [15:51] abentley: once you move the notification code here, the logic will then be correct for when we start uploading sources from recipe to Ubuntu [15:52] bigjools, actually, in order to move the code into PU.notify, I'd need to pass the build into PU.notify, so I guess I can just check whether it's None. [15:53] abentley: you don't need to pass the build [15:53] bigjools, I want to do notification by calling self.build.notify, as we do with other build notifications. [15:53] abentley: no, use PU.notify [15:53] bigjools, I don't want to duplicate the code needed to send a build notification. [15:54] otherwise it will never work properly [15:54] this is not a build notification [15:54] it's a source upload notification [15:54] bigjools, does not compute. [15:54] you are uploading a source package that has come from a recipe [15:55] all of the notification logic for that is already there [15:55] all you need to do is possibly add extra text if you know it's from a recipe (I've not seen the recipe notifications) [15:55] No, it's not. Users would get a totally different email from what they used to get. [15:55] doing this as a build notification is wrong [15:55] bigjools, why? [15:55] I just explained [15:56] where are the recipe "build" emails? [15:56] I disagree. I think upload failures should look like other build failures, except they should include the upload log. [15:57] bigjools, this is how we used to do it. [15:57] I wonder how hard it would be to add a feature to inline help to create self-documentation for enums [15:57] then they are totally different to normal source upload emails and that is inconsistent [15:57] we created the source upload format after a LOT of consultation with people [15:57] bigjools, the people who are doing recipe builds are expecting consistency with build failure emails, not source upload emails. [15:58] who are they? === beuno is now known as beuno-lunch [15:58] bigjools, upstreams. This is "bridging the gap" work. [15:58] what about successful uploads? [15:59] For recipes, successful uploads are generating build-style notifications. === matsubara-lunch is now known as matsubara [16:00] I don't think we should be doing that [16:00] it's inconsistent with source uploads [16:01] bigjools, I think we should, because it's consistent with what we were doing before async uploads were implemented, and because source upload format is not valued by the target audience. [16:01] this will cause a lot of issues when we start doing recipe builds for ubuntu [16:01] errr, yes, it is [16:02] the format is irrelevant anyway, I am telling you where to send the notifications from [16:02] you can format them how you like [16:02] or they like [16:02] bigjools, in order to format them how I like, I'm going to pass the build in. Otherwise, I'd have to omit useful information from the notification. [16:03] if you do it from the build object, you will end up duplicating all the logic in PU - as you are now discovering [16:03] ok - you'll need to pass the build in if you can't get hold of it [16:03] bigjools, no, I won't. the logic is quite different. [16:03] ok - ignore me [16:08] need a review for https://code.launchpad.net/~jml/launchpad/failing-tests-bug-711209/+merge/48184 [16:08] fix for some tests failing silently [16:09] hi jml: I will take it [16:09] sinzui: thanks. [16:14] jml: r=me [16:14] sinzui: ta [16:25] branch now in ec2 [16:25] I can hardly wait. [16:29] bigjools: ping [16:31] reviewer needs some reviews done: https://code.launchpad.net/~leonardr/lazr.restful/web-link/+merge/48052 and https://code.launchpad.net/~leonardr/launchpad/web-link/+merge/47258 [16:31] sinzui: otp, will get back to you [16:32] allenap, hi, you around? [16:32] danilos: Yes, but I need a few minutes. [16:32] allenap, sure thing [16:33] all unassigned reviews in the review queue are done [16:33] except mine [16:36] sinzui: hi, how can I help [16:37] bigjools: I am reading line 1974 of lib/lp/soyuz/doc/archive.txt. Why is the team OPEN. [16:38] bigjools, I see that isAutoSyncUpload checks both signing_key and contains_build, which suggests that they could vary independently. Are you sure checking for a signature is a reliable test, or did you mean something else? [16:38] sinzui: I suspect no reason at all [16:38] Actually I think I know. Because a MODERATED requires an extra approval step for future tests in the doc [16:38] sinzui: ah yes, good point [16:39] that doctest is a nightmare [16:39] I've watched it grow over 3 years [16:39] bigjools: so it is. I am going to change the doc to MODERATED and will update another test to use addMember [16:39] Or I can delete the entire section if I find a unittest for this behaviour [16:40] sinzui: +1 [16:40] thank bigjools [16:41] abentley: it's possible in the past that they did, but builds are never signed now [16:42] abentley: what time do you start work normally? I want to call you about this tomorrow [16:42] bigjools, I start work at 14:00 UTC. [16:42] ok, thanks [16:43] danilos: Sorry about that. My son was demanding some toy train time. What's up? [16:45] Project devel build (408): STILL FAILING in 5 hr 24 min: https://hudson.wedontsleep.org/job/devel/408/ [16:45] * Launchpad Patch Queue Manager: [r=julian-edwards][ui=none][bug=708785] Fix a trivial speeling mistak [16:45] on the 'bug also affects' popup help text. [16:45] * Launchpad Patch Queue Manager: [r=lifeless, [16:45] stevenk][bug=711016] Stop ec2 land from adding [ui=none] to commit [16:45] allenap, no worries :) [16:45] messages, since PQM no longer requires it. [16:45] * Launchpad Patch Queue Manager: [rs=thumper][bug=683798, [16:45] 669288][rollback=12277] Stop deleting recipes when people/teams are [16:45] merged. [16:45] allenap, so, I have this lovely duty of implementing LIFECYCLE direct subscriptions [16:46] allenap, I've so far implemented the way to figure out what a certain change should be notified on [16:48] allenap, like this: https://code.launchpad.net/~danilo/launchpad/lifecycle-subscription/+merge/48185 [16:49] allenap, now, I wonder how to best test for any changes in subscribers/bug.py add_notification where I want to use change.change_level directly [16:50] allenap, also, in add_bug_change_notifications there's a few getRecipients calls which all use METADATA as the level [16:51] allenap, and I am not sure how to best modify this [16:52] danilos: Did you say anything between "I've so far implemented ..." and "and I am not sure ..."? [16:52] allenap, I think I did :) [16:52] danilos: Rats, I got knocked off t'internet and bip didn't give me any scrollback. [16:52] danilos: Can you say it again? [16:53] allenap, pasted privately [16:53] allenap, under the assumption that it was you who got knocked off, and not me :) [16:53] danilos: Cool. Erm, can I look at the branch so far? [16:54] allenap, sure thing, it shouldn't be hard to deduce URL from the WIP MP :) [16:54] allenap, and in the MP, you can also see the diff so far, just to get an idea of what I am doing [16:55] danilos: You're making me work :) [16:55] allenap, there is a diff ready to consume at https://code.launchpad.net/~danilo/launchpad/lifecycle-subscription/+merge/48185 :) [16:55] allenap, and ok, lp:~danilo/launchpad/lifecycle-subscription :) === deryck[lunch] is now known as deryck [16:58] allenap, the diff so far has no changes to add_bug_change_notifications (I first want to understand how best to test this, since it seems to be involve zope event machinery or something, and events are fired "manually" in most tests I've seen) [17:00] danilos: It's taking me a while to swap this back in... :) [17:00] allenap, heh, I can understand that, you probably haven't touched this for a few years (it seems most of this code is from 2009) [17:01] danilos: Yeah, we did a sprint for this when BjornT was the team lead and I think the only thing since then is that it's moved into lp.bugs. [17:01] allenap, I am basically looking for the most minimal way to test that notifications are added properly and for the right group of people, before I go on the changing spree... [17:02] allenap, so, my idea is to basically do something like the following change: http://paste.ubuntu.com/561035/ [17:03] allenap, however, without knowing exactly what's going on, and without tests that describe how this is properly used, I don't feel comfortable JFDI [17:04] danilos: Okay. I'll try and figure something out. [17:04] allenap, heh, if you've got to dive into code yourself, I don't want to bother you, since I can do the same thing :) [17:05] danilos: I don't mind, but yes, that's what I'm going to do. I can't remember any of it without reading it again, though it might come back quickly. === beuno-lunch is now known as beuno [17:10] allenap, sure, if you can do it quickly and come up with a good attack plan, I'd appreciate it; if not, just set yourself a time limit on when to give up :) [17:17] allenap: thank you for the review! I've been looking at options for the deletion. I wanted to explore __storm_remove__ hook but I don't see any reference to that (in LP code, storm, or https://storm.canonical.com/Manual#Hooks) or anything similar, to my surprise. Unless you direct me to something I missed, I'll make "delete" or something like that. === al-maisan is now known as almaisan-away [17:17] following the pattern on the filters themselves [17:18] gary_poster: Sorry for sending you on a wild horse chase... afaik, __storm_remove__ doesn't exist. I was just thinking about it as an idea. [17:19] gotcha, allenap. A good idea :-) [17:19] but understood. I'll go with delete for now then. [17:19] Thank you! [17:19] gary_poster: Welcome :) [17:31] danilos: There are no unit tests of add_bug_change_notifications(). I think I'd start some, and simply check that the right set of recipients have notifications added for them. The notify/subscriber machinery should be tested already. It might be even clearer to stub out bug.addChange() to get the recipient list directly, and no faff around with the database. [17:32] danilos: All you really need to know is that the change_level from the IBugChange class is being used when calculating subscribers, isn't it? [17:33] allenap, yeah, but I also need to understand exactly what's going on inside that method so I don't break it as well :) [17:33] allenap, and I want to be able to state with some authority what kind of emails will people get after my changes :) [17:40] allenap, anyway, thanks for looking into it [17:41] danilos: I don't think you'll break it. For this branch I don't think you need to worry about asserting exactly what the end product looks like; the composition of the messages already tested elsewhere. You just need to check that the correct recipients are chosen. [17:42] danilos: One aside: I don't addChangeNotification() is really being used any more, except in bugnotification-sending.txt. I will throw up a branch to remove it. [17:46] the branch diff overlay on bugs pages is not removable :/ [17:52] sinzui, shall i review https://code.edge.launchpad.net/~sinzui/launchpad/team-membership-policy-1/+merge/48203 ? [18:06] allenap, thanks [18:10] leonardr: yes please [18:11] sinzui, changing the order of the enums doesn't affect which numbers they're given behind the scenes? [18:15] sinzui: i understand the difference between a delegated team and a moderated team but i don't see what difference it makes. are you going to/have you reduced the powers of people who are indirect members of a delegated team? [18:18] sinzui: was subscription_policy_description simply not used at all? the only place it's referenced here is when you remove it [18:19] leonardr: all it really does is place the new members in the proposed state. It is an open team with all the restrictions of an open team [18:19] subscription_policy_description was not used. It has not been used in 18 months [18:20] jml: merged your branch into mine and it's all hunkydory now, cheerfs [18:20] and on that note, dinner time [18:21] nn all [18:21] leonardr: locoteams is MODERATED now because they want to prevent people and unapproved teams from joining. They think it is fine if people or teams join at a local level. [18:21] sinzui: so it's an open team that requires approval for direct memberships but not indirect memberships, and there is no functional difference between a direct and an indirect membership? [18:21] leonardr: correct [18:22] sinzui: 96 "contol" => "control" [18:22] also "polcy" [18:23] ah [18:24] 166 "verses" => "versus" [18:24] * leonardr is more picky about typos in user-visible text [18:25] sinzui: i see the answer to my first question. you reordered the enums but you didn't change the numbers [18:26] damn right I did not change the enums. Those are in the database. making moderated teams into open teams will make 1000's of ppas insecure [18:27] i was just wondering how you had done that [18:28] sinzui: your split of the __doc__ on line 264 seems a little hacky to me, but it's ok [18:28] leonardr: Lp forms layout enums by their sequential order. They are easier to read and understand if the rules are changing along one axis [18:29] yeah. it is wacky. I do not like it. I could not think how to compose the enum from a title and description so that a common description could be shared. [18:32] sinzui: maybe TestTeamSubscriptionPolicyChoiceCommon.POLICY should be None? [18:32] That will break one of the tests that creates a team [18:33] never mind, i thought it was being used as a base class [18:33] I could either inline the helper logic, or I could change the helper to failover [18:34] Maybe inheritance is bad in this case because the test was ambiguous [18:34] i think it's ok [18:34] i think what tripped me up was the comment [18:35] # Any policy can be used in set of tests. [18:35] leonardr: yuck [18:35] would it be accurate to say "Any policy will work here, so we'll just pick one" [18:35] yes, that is better [18:38] sinzui: a little confused about line 598 [18:38] why is cprov now a team owner? [18:39] that is not the method sig [18:39] ITeam.addMember(member, review) [18:40] The team owner is the reviewer adding cprov [18:40] The "ignore" is a TeamMembership record for the new membership [18:41] got it [18:41] The previous code relied on the auto-approved rule in join() for OPEN teams. The problem is that no OPEN team can own a PPA, so the example documentation will actually fail in most circumstances. [18:42] i understand now. it's fallout from line 589 [18:42] sinzui: r=me with the minor changes mentioned above [18:43] thanks. my spelling will need fixing in the enum, the html help, and the wiki page because I pasted from the enums [18:43] moin [18:44] The pun never ends [18:45] sinzui: "polcy" and "contol" are only in the help afaik [18:45] maybe the wiki as well [18:47] bigjools, i will review https://code.edge.launchpad.net/~julian-edwards/launchpad/double-build-bug-705342/+merge/48219 now [18:47] hi lifeless [18:48] hi jtv [18:50] lifeless: I didn't find a good way to run external commands in parallel, so wrote up a simple manager class. Might be something you want to look at: https://code.launchpad.net/~jtv/launchpad/commandspawner/+merge/48226 [18:50] I'm going offline now though to deal with other people's IT problems. [18:51] jtv: did you look at the existing job system code for doing that? or the various twisted things we have? [18:51] jtv: we can talk later [18:52] bigjools: what's the difference between a virtual and a nonvirtual builder? [18:52] lifeless: yes, looked at those. [18:53] leonardr: a nonvirtual one we don't run kvm/xen on; its just a chroot [18:53] leonardr: policy around those is we only run trusted builds on nonvirtual builders [18:53] lifeless: you mean that the only builds run on nonvirtual builders are trusted builds? [18:54] leonardr: yes (that is, ubuntu archive builds rather than PPA builds) [18:56] lifeless: thanks. bigjools:r=me [18:58] jtv, lifeless: do you want me to take a look at https://code.edge.launchpad.net/~jtv/launchpad/commandspawner/+merge/48226 or do you want to talk it over first? [18:58] leonardr: I think you should just review it [18:58] ok [18:58] leonardr: thanks [18:58] leonardr: we certainly don't have a look-alike in-tree [18:59] there's likely something on pypi we could use [18:59] but whats written is written [19:01] jtv: if you had time in the future to either incorporate a pypi module, or to factor this out to a truely standalone thing we can publish on pypi, I think either of those things would be most excellent. [19:02] lifeless: that's a good idea, though it lacks all sorts of things that others might want but we currently don't. [19:03] jtv: sure; we can accept patches for such things :) [19:03] "the Internet will solve it!" [19:03] https://github.com/taichino/python_prefork is similar [19:03] Oh, sorry, that's Internet 1.0. The 2.0 phrase is "we'll crowdsource it." [19:04] * jtv mumbles something about not needing not steenkin' github projects [19:04] * jtv instantly regrets this as github sends chromium into an unusable state [19:06] oh oracle [19:06] you sad cats [19:07] lifeless: ? [19:07] you know about the hudson trademark thing, right ? [19:07] Not enough, evidently. [19:07] ok, so oracle buy Sun [19:08] yes me knoooo [19:08] me hold share [19:08] but cats? [19:08] day one they have the hudson lead developer go from a decent work environment - good sized dual lcd yada yada yada, to a puny single screen in a existing oracle block [19:09] not long later he walks [19:09] then this trademark thing brews up [19:09] community voted - 90 [19:09] 90% wanted to rename to get out from under the trademark. [19:10] overnight a 'susan duncan' from oracle posted a 'hudson will continue as hudson, we have a team @ oracle, come one come all' kind of email. [19:10] so rather than following 90% of the devs + users, they are going alone, with -1- person that knows the codebase at all. [19:10] (and who wrote ~ 1% of it) [19:11] * jtv nods attentively [19:13] lifeless: so were you using the word "cats" strictly in the sense of "people"? [19:13] yes, getting my 70's thang on [19:14] Now I see. [19:14] So yes, sad cats indeed. [19:15] How about we pass the hat around and see how much we can offer Michael to change his name? [19:15] lol [19:15] The prefork thing looks like it would't do much for our use case by the way, though it's hard to say without documentation. [19:16] its not a direct fit [19:16] thats for sure [19:18] jtv: lol; the first two replies to her message: 'unsubscribe' [19:18] [19:20] Now if they could re-do it in python, that'd be different. [19:21] sonatype are going with hudson [19:21] cloudbees with jenkins [19:22] wheee [19:22] jtv: can you make it clear in line 287 that error 11 *is* "resource temporarily unavailable"? i had to look it up to make sure [19:22] leonardr: oh, I thought I'd done that. Will fix fortwith. [19:22] forthwith. [19:22] jtv: it's just a little vauge [19:24] leonardr: You're right. Found a better spelling: errno.EAGAIN! [19:27] Fix pushed. [19:33] leonardr: you may be wondering why, in communicate(), I didn't extract the loop body into a method. I did that originally, but that hid the fact that I remove an entry from the dict that the loop iterates over—and therefore the reason to iterate over keys() instead of iterkeys(). [19:34] jtv: do you need to iterate over dead processes once they're dead? [19:35] No, I just don't want to upset the iteration. [19:35] i don't think it matters much, but do i have it right that it's not necessary? [19:35] Yes, you have it right. [19:35] Maybe I'm just being too conservative. [19:35] i was wondering about test_communicate_returns_after_event [19:36] what happens to the sleep process? it just keeps running in the background? [19:36] would it make sense to tear that down? [19:36] if it's running in the background it's not a big deal [19:37] It does, but that's why I did the self.addCleanup(spawner.kill). [19:38] thanks, i was looking for a TestCase style teardown [19:38] ie an actual teardown method in the class [19:38] Though come to think of it… it'd be neater to do actual wait()s for the child processes wouldn't it? [19:39] i don't know much about this--is wait() blocking? [19:40] Technically it is, I think, but not for long. [19:40] It's been ages, so I'm vague on the details… wasn't un-wait()ed children how you got zombies? [19:40] jtv, i am also vague on the details [19:41] I can whip up a quick test to find out. [19:43] Yup, that's a zombie. Need to wait() for it. [19:44] jtv: will that cause the test to stall for 10 seconds? [19:44] No [19:44] Just keep a zombie process around. [19:44] Which is basically garbage and not nice, [19:45] jtv: i mean if you wait() will it cause the test to stall [19:45] No, only if the sub-process catches the terminate signal. Which doesn't happen in the tests. [19:50] morning [19:51] morning thumper! [19:51] jtv: up late? [19:51] thumper: no, in NL [19:52] leonardr: I'm just pushing a change that does a kill()/complete() on all spawners in the acceptance test. That oughta take care of any zombie danger. [19:53] i'm glad i could stumble into an improvement [19:53] * thumper goes to look at the vast amount of email in the inbox [19:54] leonardr: so am I—I'm not a big believer in permissive reviews. ☺ [19:56] taking a look at your changes now [19:56] ~/win 63 [19:58] I've heard of win64, but… [20:01] jtv, r=me [20:01] leonardr: thanks! I'm adding a docstring note to explain about the zombies, but otherwise, should be good to go. === lifeless changed the topic of #launchpad-dev to: https://dev.launchpad.net/| PQM is open | firefighting: - | On call reviewer: leonardr | https://code.launchpad.net/launchpad-project/+activereviews [20:02] ok [20:03] deryck: ping [20:03] flacoste: can you high-pri rt 43756 ? [20:03] flacoste: I have to run to town with Lynne, back in ~3 hrs [20:03] deryck: https://code.launchpad.net/~thumper/lazr-js/multi-line-editor-goodness/+merge/48118 [20:03] * leonardr could still use reviews of two of his branches [20:03] leonardr: https://code.launchpad.net/~thumper/lazr.restful/only-traceback-system-errors/+merge/48075 [20:04] leonardr: what do you need? [20:04] thumper: hi there [20:04] leonardr: I started on one [20:04] leonardr: the other was still work in progress [20:04] thumper, I didn't have time to look yet. You wanting me to review it for you? Or just see what's up? [20:04] leonardr: although I did look through it [20:04] yeah, i responded to it. the other one is ready now [20:04] deryck: I'm after a real review :) [20:04] the other one being https://code.launchpad.net/~leonardr/launchpad/web-link/+merge/47258 [20:04] deryck: but pointers to what testing may be needed [20:04] thumper, looking at your lazr.restful branch now [20:05] lifeless: no permission to view ticket, did you file using launchpad@rt... ? [20:05] deryck: the only JS thing that has really changed is the stripping of trailing whitespace, and hiding the spinner [20:05] thumper, ok, give me a minute, and I'll try to get it before I EOD. [20:05] deryck: thanks [20:05] np [20:05] flacoste: jam filed it [20:05] jam: ^ [20:05] flacoste: gone; back later [20:07] thumper: r=me [20:07] leonardr: your lp weblink branch looks fine to me [20:07] leonardr: shall I mark it needs review? [20:07] g'night. [20:07] night jml [20:07] thumper: yeah, go ahead. i didn't realize that was what was keeping it [20:08] ok [20:09] gary, i am reviewing https://code.launchpad.net/~gary/launchpad/bug711362/+merge/48232 [20:12] thank you leonardr [20:14] gary, i am confused by lambda: self.subscription.delete instead of self.subscription.delete. what's the difference? [20:15] is it the very attempt to access the delete attribute that causes the exception? [20:15] hmm... I need to configure my local bits to be able to sumbit to lp:lazr.restful [20:15] leonardr: yeah, I tried that first too. yes, exactly [20:16] thumper: do a bzr co of trunk, then merge in and bzr commit. that's what i do [20:16] leonardr: it isn't pqm managed? [20:16] thumper: no [20:16] leonardr: ah... ok [20:16] * thumper does that [20:17] we could maybe make it pqm managed again, but it used to be submitting a change to lazr.restful ran the launchpad test suite for no reason [20:18] thumper, so looks good to me. I would add a js test that the icons are re-enabled on error. And some text on the example page to explain that it will error every other attempt would be nice. [20:18] thumper, r=me with those changes. [20:18] deryck: ok [20:18] gary: what do you mean "The original delete fails if it does not remove the filter first"? i don't see that in the code. do you mean that the only way the filter could not be removed is if the delete failed? [20:19] leonardr, benji: any news on the packaging of the new desktop integration into natty? [20:19] flacoste: no, i'll ping barry again [20:19] leonardr: thx [20:21] leonardr: the DB rejects the delete of the structural subscription because the database would then be an inconsistent state [20:21] (with the filter record pointing to the structural subscription) [20:22] ok, in that case i'm not sure what 'the original delete' is [20:23] leonardr: it removes the filter first in the delete method. I think "the original delete" is a bad term at least in part because there is no need to call it "original". I guess I meant something like this: [20:23] wow... the second buildout of lazr.restful was much faster than the first [20:23] thumper: that's the cache advantage [20:25] "Trying to delete the structural subscription without first deleting the filters, as the "delete" method does, would raise a database IntegrityError. Therefore, we know that the filter really is gone, but we double check it here anyway." [20:26] leonardr: and fwiw, this is the exception you would have gotten: [20:26] IntegrityError: update or delete on table "structuralsubscription" violates foreign key constraint "bugsubscriptionfilter_structuralsubscription_fkey" on table "bugsubscriptionfilter" [20:26] DETAIL: Key (id)=(5) is still referenced from table "bugsubscriptionfilter". [20:26] leonardr: lazr.restful branch landed \o/ [20:26] leonardr: much faster than PQM :) [20:26] "as the delete method does" is still confusing :-/ [20:26] gary: ok, that's what i thought was going on. how about this wording [20:27] "We know that the filter is gone, because we know the subscription is gone, and the database would have prevented the deletion of a subscription without first deleting the filters" [20:28] "but let's double check" [20:28] yes, leonardr [20:28] thank you [20:28] gary, np [20:29] gary, r=me [20:29] Thanks leonardr. I'm improving the comment now. [20:29] I'll also add a comment about the lambda [20:30] thumper: once my ec2 test completes i will do a lazr.restful release and land the launchpad branch [20:30] leonardr: sounds great [20:30] i was able to verify that launchpadlib can access web_link if it's present [20:30] leonardr: my editor work has to wait for that to land first [20:30] great [20:30] thumper, chat? [20:31] abentley: sure [20:31] abentley: mumble? [20:31] thumper, yep === matsubara is now known as matsubara-afk [20:52] flacoste: I just used rt@admin.canonical.com, was I supposed to do something different? [20:52] jam: yes, please use launchpad@rt.canonical.com, it puts it in the right queue automatically [20:53] flacoste: k [20:53] that's the first I've heard of that address [21:00] thumper: got some test failures. some very minor, one possibly serious [21:00] leonardr: oh? [21:00] thumper: minor ones are tests that had 'launchpad.net' urls instead of 'bugs.launchpad.net' for cves and so on [21:01] ok [21:01] wallyworld_: https://code.launchpad.net/~thumper/lazr-js/multi-line-editor-goodness/+merge/48118 [21:01] the possibly serious one is a launchpadlib test that systematically checks every _link parameter as though it were a link to another launchpad web service object [21:01] basically, it sees web_link and thinks there must be a corresponding 'web' property [21:04] hmm... [21:04] if it is just a test, surely we could add an exception [21:04] leonardr: mumble? [21:04] StevenK: mumble ping [21:04] thumper, sure [21:05] thumper: Coming, sorry [21:05] sinzui: you around? [21:06] I am [21:06] sinzui: are you able to sign off on: https://code.launchpad.net/~wallyworld/launchpad/recipe-find-related-branches/+merge/47367 [21:06] pretty please :-) [21:06] ui review [21:07] wallyworld_: You have my approval [21:08] sinzui: thanks! much appreciated [21:11] abentley: Did you end up reaching agreement with bigjools about where to do notifications? [21:12] wgrant, no, and I am handing off the project to thumper. [21:12] abentley: Well, from the snippets I read he argued that PU should do the notification. I am adamant that he is wrong, FWIW. [21:13] wgrant, where do you think it should be done? [21:14] abentley: Build.notify, as it always has been. [21:16] wgrant, cool. [21:29] thumper: first off, the test failure [21:29] yep... [21:29] i believe the correct thing to do is to change lazr.restfulclient to interpret 'web_link' as a scalar value, not a link, when producing lists like lp_attributes and lp_entries [21:29] that's an easy change to make but it's not easy to test [21:30] so i suggest we file a bug for it, hack similar behavior into the launchpad tests, and land what we have [21:31] leonardr: I'm not entirely clear on the underlying meaning in lazr.restfulclient with respect to the lp_attributes and lp_entries [21:31] and how the web_link fits into it [21:31] is it that web_link will move from lp_entries to lp_attributes? [21:31] thumper: yes, to put it slightly more accurately, 'web' will move from lp_entries to lp_attributes and will become 'web_link' [21:32] leonardr: that sounds fine to me [21:32] lazr.restfulclient thinks that anything ending in 'link' is a link to another object comprehensible by lazr.restfulclient [21:32] in this case it is a link, but not one comprehensible by lazr.restfulclient [21:32] right [21:32] I think special casing it is fine [21:32] it doesn't have to be all that special [21:33] we can distinguish ahead of time between comprehensible links and incomprehensible, we've just never had incomprehensible links before [21:33] how do we determine that it is incomprehensible? [21:34] thumper: by looking at the wadl. web_link has a tag whose element is empty--it doesn't point elsewhere in the wadl [21:34] in the words of the test failure error, "Parameter is a link, but not to a resource with a known WADL description." [21:34] ok, is this part of the quick fix of the longer bug fix? [21:34] no, this is the longer bug fix--the problem is testing it [21:35] we might be able to fake this, but i believe we need to change lazr.restful so that the example web service publishes web_link (right now web_link only shows up in unit tests) [21:35] so to be clear, we can make the fix quickly, and file a bug for the testing of it? [21:35] thumper: i was suggesting fixing it in launchpad, but yeah, we can fix it for real pretty easily [21:36] leonardr: does the restfulclient *need* lazr.restful example service to publish web_link? [21:36] I would think that a fake would be better [21:36] thumper: that's how we do the tests. i would prefer a fake as well, but i don't think we have the infrastructure for it [21:37] ah.. the tests for lazr.restfulclient depend on the lazr.restful example server? [21:37] yes, they are integration tests making simulated http requests and getting responses [21:37] we can do unit tests in lazr.restful but not so much in the client, i think [21:38] I'm thinking of a potential solution... [21:38] it may be completely off though [21:39] let's hear it [21:39] what if, as part of the tests for restfulclient, we had a decorated server, that intercepts the calls to the example restful server and annotates it with a web link? [21:39] thumper: it would be simpler to make the example server serve a web link for real [21:40] probably [21:41] actually, i could probably work something up with a fake wadl document [21:41] * thumper likes fakes (of a certain kind) [21:41] that would be closer to a unit test [21:42] since there is no data going over the wire [21:42] it's just restfulclient making judgements about the structure of the web service based on the wadl [21:43] barry: lazr.restfulclient in natty is up-to-date [21:43] so don't worry about that [21:46] wgrant: Have we seen the test_404 and one other failures in ec2 before? [21:46] thumper: two possibilities, your call [21:46] StevenK: i've seen them once. [21:46] And they've appeared on Hudson [21:46] But they only appear often on buildbot. [21:46] * thumper nods [21:46] 1. i'll fix the failing tests in launchpad, get my branch into ec2 land, and fix it in restfulclient tomorrow [21:46] 2. i'll fix it in restuflclient tomorrow and land everything tomorrow [21:46] wgrant: What do you think? Resubmit and hope ec2 is kinder? [21:47] leonardr: what breaks if we go for 1 ? [21:47] StevenK: I'd just go to lp-land if they were the only failures. [21:47] leonardr: just test failures in lazr.restfulclient? [21:47] thumper: launchpad contains code to work around the problem for a little while [21:48] leonardr: what is the impact of that though? [21:48] thumper: not much. just "this is hacky" [21:49] leonardr: lets land everything when it is good to work together [21:49] thumper: ok, that will be my goal for tomorrow [21:49] ok [21:49] i'd like to hear your suggestions for what to do after that [21:50] leonardr: i was looking at the lazr.restful errors [21:50] well, bugs, not errors [21:50] right [21:50] bug 619180 is listed as critical, and probably not too much [21:50] <_mup_> Bug #619180: UnicodeEncodeError creating a team that contains non-ASCII on name via API < https://launchpad.net/bugs/619180 > [21:51] also, bug 373370 looks like it should be a good to do candidate [21:51] <_mup_> Bug #373370: Text fields can be made empty via the API, but not via the web UI < https://launchpad.net/bugs/373370 > [21:51] thumper: ok, i'll take a look. may be lazr.restful, may be launchpad [21:51] leonardr: understood [21:52] leonardr: Thanks for approving my branch. [21:52] huwshimi: np [21:52] Any idea why staging is out of date? [21:52] leonardr: the example given in bug 619180 actually has unicode in the name, which would hit a validation error [21:52] <_mup_> Bug #619180: UnicodeEncodeError creating a team that contains non-ASCII on name via API < https://launchpad.net/bugs/619180 > [21:52] There's nothing in the logs since the last update finished several hours ago. [21:52] leonardr: I wouldn't expect a unicode encode error though [21:53] my guess is that it should be a 400 Bad input type error rather than an oops [21:53] Hmm. It's possible that it's a Launchpad issue, and the exception's __str__ is trying to do bad things. [21:54] wgrant: could be [21:54] thumper: that says to me that validation code is not running, since errors during validation are turned into 400s [21:54] leonardr: This seems to be a UnicodeEncodeError while reporting the validation failure, right? [21:55] leonardr: perhaps with our change several weeks ago, to fix the validation, it may now fail better :) [21:55] thumper: I reproduced it when I reassigned it. [21:55] It's still present. [21:55] wgrant: from when? [21:55] i guess i should have looked more carefully at the traceback [21:55] yeah, it's an error presenting the validation error [21:56] thumper: Hm? I reproduced it like 3 days ago. [21:56] could be === almaisan-away is now known as al-maisan [22:06] wgrant: Did you tag the other bug too? [22:12] StevenK: Yes. [22:12] To unbreak qa-tagger. === al-maisan is now known as almaisan-away [22:12] wgrant: Yes, I just got the mail, thanks. [22:12] It needs bad-commit-* for the rollback to be effective. [22:12] abentley: is there a bug for this? [22:12] I had to read the code to find out why it wasn't working :/ [22:13] wgrant: :-/ [22:13] wgrant: Aside from Henning, it looks like your QA [22:15] thumper, you mean bug #681125? [22:15] <_mup_> Bug #681125: Async uploader sends recipe mail as if it were source package uploads < https://launchpad.net/bugs/681125 > [22:15] StevenK: Yup. [22:15] StevenK: But qastaging's codebrowse is either broken or not there, and staging is one rev too old. [22:15] abentley: yep, thanks [22:16] wgrant: So tempted to reply 'lolwut' [22:17] abentley: To be clear, your recipe mail changes at the head of the deployment queue are still OK to roll out? [22:17] Despite the current debate? [22:17] wgrant, yes. My current changes are deployable, but an incomplete fix. [22:18] Great, thanks. === leonardr changed the topic of #launchpad-dev to: https://dev.launchpad.net/| PQM is open | firefighting: - | On call reviewer: - | https://code.launchpad.net/launchpad-project/+activereviews [22:31] wgrant: mumble? === StevenK changed the topic of #launchpad-dev to: https://dev.launchpad.net/| PQM is open | firefighting: - | On call reviewer: StevenK | https://code.launchpad.net/launchpad-project/+activereviews [22:44] sinzui: you want to continue on Person:+bugs, or just catch up tomorrow? [22:45] wgrant: Your mumble stutters quite a bit for me, but everyone else sounds fine. Is everyone clear for you? [22:45] huwshimi: Hm... Everyone's clear for me, and I sounded clear enough from the feedback I heard through sinzui's mic. [22:46] Odd. [22:46] wgrant: yeah that is odd [22:47] wgrant: are you testing rev 12290? - it's currently blocking subsequent rollouts [22:47] sinzui/jcsackett: Was my mumble connection OK? [22:47] wallyworld_: I am waiting on a LOSA to unbreak qastaging codebrowse. I have a nodowntime rollout request for 12293 in my clipboard :) === salgado is now known as salgado-afk [22:48] wgrant: thanks :-) just trying to get ready for pqm closure on friday [22:52] wgrant: yeah, you sounded fine. [23:13] sinzui: I fixed my css issue [23:13] sinzui: How do I go about using the sprites? [23:55] huwshimi: drop the new image into lib/canonical/launchpad/images . Add a new class to lib/canonical/launchpad/icing/style-3-0.css.in (the comment is required) [23:58] huwshimi: run make sprite_image then make css_combine to see your change [23:58] StevenK: Could you review https://code.launchpad.net/~wgrant/launchpad/bug-654585-linkify-bug-acknowledgement/+merge/48254? [23:58] Can I get bzr di to give more context with it's diffs? The manual page and bzr di --help weren't very helpful [23:59] StevenK: I don't think it can do it directly, but you could use --diff-options.