| === heroux_ is now known as heroux | ||
| === danilos` is now known as danilos | ||
| cjwatson | The review queue is getting rather backed up ... | 10:18 |
|---|---|---|
| wgrant | Hm, just the one from Thursday and five from Friday, or am I missing some? | 10:33 |
| cjwatson | Those, I think. | 10:40 |
| cjwatson | Oh, and ~cjwatson/turnip/merge-prerequisite, and a recheck of my recent changes to ~cjwatson/launchpad/team-mail would be good since those introduce async jobs. | 10:41 |
| wgrant | Ah, I see, will do. | 10:41 |
| cjwatson | Ta. I'm about to tackle the immediate mail delivery thing in zopeless, at least moving things along a bit. | 10:41 |
| wgrant | Hm, which thing is that? | 10:42 |
| cjwatson | https://bugs.launchpad.net/launchpad/+bug/29744 | 10:42 |
| mup | Bug #29744: In Zopeless, mails should be sent only when the transaction is commited <email> <infrastructure> <lp-foundations> <tech-debt> <Launchpad itself:Triaged> <https://launchpad.net/bugs/29744> | 10:42 |
| wgrant | That's a huge amount of trouble. | 10:42 |
| wgrant | Due to archiveuploader relying on them being sent on abort | 10:42 |
| cjwatson | Not terrible to at least make it more opt-in. | 10:42 |
| wgrant | IIRC | 10:42 |
| wgrant | True. | 10:42 |
| cjwatson | I've almost fixed archiveuploader. | 10:42 |
| wgrant | Ooh | 10:43 |
| cjwatson | But the first step is to make it easier for archiveuploader to opt-in selectively. | 10:43 |
| cjwatson | Actually the first first step is to convert more stuff to pop_notifications, because otherwise there's a pattern that's rather too easy to fit where you do self.assertEqual([], stub.test_emails) without committing first, and moving away from immediate delivery makes that kind of test silently ineffective. | 10:43 |
| cjwatson | I have a fully passing test suite (with the exception of some unrelated buildmaster cruft) with immediate delivery disabled in zopeless in general but enabled in BaseMailer and a few other places. | 10:44 |
| cjwatson | The reason I care is that otherwise converting the world of mail notifications to zopeless (so that we don't have to care about having an actor who might not be allowed to know about some subscribers) might be a rather more exciting stealth change than it ought t obe. | 10:46 |
| wgrant | I'm not too concerned about that. | 10:47 |
| wgrant | It is a slight issue, but not very large. | 10:47 |
| wgrant | As the main thing that can fail is operation itself, not the email transmission. | 10:47 |
| wgrant | Good to fix, anyway. | 10:47 |
| cjwatson | archiveuploader is relatively easy now that most of the code is in PackageUploadMailer. The fix is to use the mailer directly rather than relying on going through PackageUpload.notify; then it doesn't have to abort in order to get rid of the temporary PU object, and can just do abort; send mail; commit | 10:47 |
| cjwatson | Requires pushing a couple of extra bits of logic down into the mailer to avoid duplication, but not very much. | 10:48 |
| wgrant | Yep | 10:48 |
| cjwatson | And that self.queue_root.notify business in archiveuploader was something I always hated. | 10:49 |
| cjwatson | Ha, I found something that was still depending on the "debian" alias for binarypackage! | 10:54 |
| wgrant | I believe it works due to a very old launchpad-buildd on the buildbot slaves. | 10:55 |
| cjwatson | Yep. But they have at least 121 per my IRC logs, so we can use the new forms. | 10:56 |
| cjwatson | I'll get a clean bin/test locally yet. | 10:57 |
| wgrant | Oh, I didn't realise they were that new. Handy. | 10:57 |
| wgrant | Hmm | 11:03 |
| wgrant | We should really stop catching IntegrityError in places. | 11:03 |
| cjwatson | aaaa | 11:04 |
| wgrant | Only Packageset, Snap and LiveFS do it still. | 11:04 |
| wgrant | What triggered the IPv6 alarm? | 11:04 |
| cjwatson | Oh, yeah, I forget why I couldn't think of a better way to do that | 11:04 |
| cjwatson | Just fear at what else that could hide | 11:04 |
| wgrant | It just had me running in circles for a bit. | 11:04 |
| cjwatson | Though it is indeed my fault in this case | 11:04 |
| wgrant | Chasing down why a snap got created despite the feature flag being disabled and causing a 401. | 11:05 |
| wgrant | Of course it was actually lying when it said one existed -- instead, git_path was null. | 11:05 |
| wgrant | The correct thing to do in this situation is LBYL | 11:05 |
| cjwatson | I was about to say exactly that! | 11:05 |
| cjwatson | We're raising a specific error so should check for a specific condition. | 11:05 |
| wgrant | Yep | 11:05 |
| wgrant | I think you actually copied that originally from a place that I fixed reasonably recently... | 11:06 |
| cjwatson | Almost certainly. | 11:06 |
| cjwatson | Yay clean buildmaster tests | 11:06 |
| wgrant | Marvellous. | 11:07 |
| cjwatson | I believe that's an entirely clean bin/test for me locally now, with the codeimport fixes. I ran the whole thing at the weekend to see what exploded without immediate delivery. | 11:07 |
| cjwatson | 3.5 hours | 11:08 |
| wgrant | Ah, that's right, LibraryFileAlias.create said a filename contained illegal characters whenever there was an IntegrityError. | 11:08 |
| wgrant | Not quite the same, but close. | 11:08 |
| wgrant | Yep, even Haswell-U is pretty snappy at running the test suite nowadays. | 11:09 |
| lifeless | cjwatson: 3.5 *hours* ? | 19:23 |
| cjwatson | lifeless: In which direction is that surprising? :) | 19:58 |
| lifeless | cjwatson: seems slow - I recall running it locally in ~90m ? | 20:04 |
| lifeless | cjwatson: (with parallel of course) | 20:04 |
| cjwatson | lifeless: This is just serially | 20:04 |
| cjwatson | No particular effort to make them go faster, I was doing housework for those 3.5 hours :-) I very rarely run the whole thing locally, this is the first time it's been necessary for me in eight months full-time on LP | 20:05 |
| cjwatson | If I did it more often I expect I'd bother setting up the parallel harness stuff | 20:05 |
| lifeless | cjwatson: ah. Fair enough ... the parallel setup is pretty easy, and it helps even with running subsets | 20:06 |
| cjwatson | It's probably worth an hour of effort at some point | 20:06 |
| cjwatson | Effort in that direction would be more appealingly spent on being able to run tests on more than one branch at once, mind you (though I suppose it involves some of the same work) | 20:07 |
| lifeless | huh, what stops you running on more than one branch? | 20:08 |
| lifeless | presumably the only thing is the template db | 20:08 |
| lifeless | and thats (IIRC) controllable via an env variable. | 20:08 |
| cjwatson | Never having bothered to work out the details :-) | 20:09 |
| cjwatson | There are various specific tests that are singleton in odd ways | 20:09 |
| cjwatson | Like starting up servers on hardcoded port numbers | 20:09 |
| lifeless | pretty sure all the fixtures report back the port number | 20:10 |
| lifeless | if not, I think thats likely a regression | 20:10 |
| cjwatson | Pretty sure not everything uses them right | 20:10 |
| lifeless | entirely possible :) | 20:10 |
| lifeless | anyhow, yes the work doth likely overlap | 20:10 |
| lifeless | but - the container approach gives you entire isolation anyhow | 20:10 |
| cjwatson | Like the buildmaster integration tests have AIUI never done dynamic port selection | 20:10 |
| cjwatson | So a few tests fail obscurely if you happen to have a real buildd running in the same environment | 20:11 |
| lifeless | so if you have parallel, you have multibranch with at most trivial glue | 20:11 |
| cjwatson | Yeah. One of these days ... | 20:11 |
| lifeless | indeed http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/view/head:/lib/lp/buildmaster/tests/harness.py | 20:14 |
| lifeless | is missing port randomisation | 20:14 |
| lifeless | there's glue for that elsewhere in the three. TUITs huh | 20:15 |
| cjwatson | There's lib/lp/services/librarianserver/testing/server.py | 20:16 |
| cjwatson | I think that's the only bit of .tac glue that does it | 20:17 |
| cjwatson | log parsing, nasty | 20:17 |
| lifeless | yes | 20:18 |
| lifeless | well | 20:18 |
| lifeless | its better than the race of the 'find a port in parent and say 'use port X' on the command line | 20:19 |
| lifeless | an actual fd handover would arguably be better | 20:19 |
| lifeless | log parsing is at least race free | 20:19 |
| cjwatson | Yeah | 20:19 |
| cjwatson | Would want to try to push that down a level if using that from more places | 20:20 |
| wgrant | lifeless: There are heaps of things that were never isolated | 23:46 |
| wgrant | That's why the container approach was used,. | 23:46 |
| wgrant | eg. AppserverLayer even conflicts with the dev appserver. | 23:47 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!