[06:58] <al-maisan> Good morning
[07:36] <noodles775> gmb: I'm just updating those comments in the js-test - you're right, I can see how it's now much easier to read as a dev. :)
[09:06] <noodles775> allenap: I'll have one ready for you in 5 :)
[09:07] <allenap> noodles775: Jolly good :)
[09:32] <noodles775> allenap: thanks! https://code.launchpad.net/~michael.nelson/launchpad/432152-orig-file-counts/+merge/12645
[09:32] <allenap> noodles775: Cool.
[10:16] <allenap> noodles775: I don't understand the query really. It reads like you're selecting distinct LFAs.filename where the corresponding content is the same size.
[10:17] <allenap> noodles775: I don't understand how that can be a valid thing to ask for, so maybe that's explained by the other clauses in the query.
[10:19] <noodles775> allenap: I'm just wanting distinct filenames to then sum the size - it just so happens that when the lfas with the same filename will also have the same size.
[10:19] <noodles775> allenap: the unit test might help to demo what it's doing?
[10:19] <allenap> noodles775: I should probably read it :)
[10:30] <allenap> noodles775: If I apply http://pastebin.ubuntu.com/282003/ to your branch, the tests still pass (note the differing filecontent). Should that happen?
[10:31]  * allenap will be back in ~5-10 minutes.
[10:31] <noodles775> allenap: yes, that's correct.
[10:32] <noodles775> allenap: it should fail if you change the name.
[10:32] <noodles775> allenap: ah, I see... the filecontent is the actual content, not the length, so if you use filecontent='222' (ie. 3 bytes) it should also fail.
[10:42] <allenap> noodles775: When does it not matter if the content is different but the size the same?
[10:42] <noodles775> allenap: the query is only checking the filesize - not the filecontent.
[10:43] <noodles775> For publishing tarballs/dsc files, (according to cprov), the fact that the filename is the same is enough to ensure that they are the same file (in that archive).
[10:43] <allenap> noodles775: Ah, okay, that's what I was looking for :)
[10:43] <noodles775> But it might make it clearer if we also include the LFC.sha1 in the query, so it's obvious that that's what's happening (I tried to clarify that in the comment, but it's still confusing).
[10:45] <allenap> noodles775: Yeah, including the sha1 will probably document this better, and will also mean that future developers don't suspect a smell when there isn't one because of the data model.
[10:46] <noodles775> allenap: great, I'll do that then.
[11:02] <allenap> noodles775: Thanks, r=me, with one other suggestion, which you are welcome to ignore if you don't like it.
[11:02] <noodles775> Thanks allenap !
[12:06] <stub> allenap, jtv: https://code.edge.launchpad.net/~stub/launchpad/librarian-gc/+merge/12650 , https://code.edge.launchpad.net/~stub/meta-lp-deps/updates/+merge/12322 , https://code.edge.launchpad.net/~stub/launchpad/sanitize-db/+merge/12520
[12:06] <stub> (in order both smallest to largest and in priority too)
[12:06] <jtv> just when the lunch gong goes...
[12:06] <jtv> allenap: shall I take that last one then?
[12:07] <allenap> stub: Is the first the highest or lowest priority?
[12:07] <stub> first is highest since it might need to be cherry picked.
[12:08] <allenap> jtv: Perhaps you should do the first then.
[12:09] <jtv> allenap: okay
[12:09] <allenap> jtv: I am about to go to lunch too :) But I'll pick up as soon as I'm back.
[12:10] <allenap> Sorry stub, looks like both of us are afk now. Nothing to do with you, well, on my part at least :)
[12:10] <stub> k
[12:25] <thumper> sinzui: https://code.edge.launchpad.net/~thumper/launchpad/rehash-branch-index/+merge/12651
[12:29] <jtv> stub: I'm surprised to hear that the librarian-gc branch fixes anything (unless content_id is None as well), since AFAIK None < anything
[12:30] <stub> None < some_integer == True
[12:30] <stub> We don't want that test to pass if next_wanted_content_id is None
[12:31] <jtv> stub: oic
[12:31] <stub> Or we would be reporting 'LibraryFileContent None exists in the database', or we would if it didn't crash when trying to format None as an integer.
[12:34] <jtv> stub: I can't help feeling that that loop would be a lot prettier with the config check outside it, but if this is for CP...
[12:34] <jtv> It would be good to have a test though.
[12:36] <stub> jtv: Hmm.... I don't see that working as I still need to lookup the next wanted id even if there is an upstream librarian
[12:37] <stub> I was hoping nobody noticed the lack of test ;)
[12:38] <stub> Now I have to work out what actually triggers this case :)
[12:38] <stub> I think it occurs if the highest id isn't wanted
[12:38] <jtv> it can avoid disappointments...
[12:39] <stub> (Hmm.... that should be a generator really)
[12:45] <jtv> stub: to get me into the right context: is this a CP candidate?
[12:46] <stub> jtv: Yes
[12:46] <stub> I can try adding a test in a minute
[12:47] <jtv> stub: I'd appreciate that
[12:57] <stub> Oh sod off. I know there is another mp for this branch.
[12:57] <stub> facist review system :-P
[12:58] <stub> Can http://paste.ubuntu.com/282095/ be reviewed without a mp ?
[13:03] <jtv> stub: you could stick a tinyurl on the queue
[13:03] <stub> np. worked out how to do the resubmit.
[13:04] <stub> https://code.edge.launchpad.net/~stub/launchpad/memcache/+merge/12653
[13:05] <jtv> stub: that second branch has a review request for Francis... do I take it you don't want to wait for that?
[13:05] <stub> If you know anything about debian packaging, sure.
[13:06] <jtv> stub: rusty, but this looks pretty trivial.  Why don't you tell me what could go wrong?
[13:06] <stub> I just assigned francis as he suggested this workflow and it sat there for a few days ignored ;)
[13:06] <stub> jtv: I have no idea what could go wrong. I don't even know what happens after it gets merged in to build the package ;)
[13:08] <jtv> stub: well my apt has no objections to installing memcached, so you've got my imprimatur
[13:09] <jtv> stub: any progress on test?
[13:09] <stub> jtv: Nah - reject it and I'll resubmit when I have something
[13:09] <jtv> stub: as you wish
[13:20] <noodles775> allenap, jtv: just in case you get to it before I'm back, my MP is here: https://code.launchpad.net/~michael.nelson/launchpad/436182-newlines-in-sources-bz2-indices/+merge/12655
[13:20] <allenap> noodles775: Cool.
[13:22] <jtv> noodles775: in call
[13:22] <jtv> will get back later
[13:56] <deryck> I want a ☎
[13:56] <deryck> allenap, hi
[13:57] <allenap> deryck: Hi
[13:58] <deryck> allenap, I have a small branch, but can I leave it on the queue for offline review.  About to return to tl sprint stuff.
[13:58] <allenap> deryck: Okay, cool.
[13:58] <deryck> allenap, thanks!
[13:59] <deryck> allenap, never mind.  gpg fail on new laptop setup.  will catch up later about it.
[13:59] <allenap> stub: Does ALTER TABLE not implicitly commit a transaction?
[13:59] <allenap> deryck: Heh :)
[14:14] <stub> allenap: No. PG is entirely transactional that way.
[14:28] <jtv> allenap: which one for stub is still on the queue?
[14:28] <allenap> jtv: https://code.edge.launchpad.net/~stub/launchpad/memcache/+merge/12653 I think
[14:28] <jtv> allenap: ok
[14:36] <jtv> stub, about the memcached test: why not self.assertEqual(self.want_memcached, is_live)?
[14:37] <stub> jtv: Better error messages
[14:40] <jtv> I guess...
[14:41] <jtv> Something that bugs me about layers: can't we have split-transaction startup & teardown?
[14:42] <stub> What do you mean?
[14:43] <jtv> So e.g. memcached can start gearing up before Librarian has completed its startup, or vice versa.
[14:43] <jtv> Put those extra cores to work.
[14:43] <stub> jtv: Not out of the box.
[14:44] <jtv> Shame.
[14:44] <jtv> On a different note: in Memcachedlayer.setUp, you assign test_key twice, sort of defeating the purpose.
[14:44] <stub> jtv: We could do it ourselves if we wanted - launch stuff in separate threads and block until they are all ready, but nobody has tried that yet.
[14:44] <stub> I did?
[14:44] <jtv> Well somebody did
[14:45] <stub> fixed
[14:45] <jtv> stub: http://paste.ubuntu.com/282095/ lines 150 & 168
[14:47] <jtv> I'm not a huge fan of firing off threads for this sort of thing, since it gets hard to manage.  But why sit around looping for your service to become available when you could just return and let the next service start?  Managing dependencies is a bitch, of course.
[14:48] <stub> I wouldn't want to bother until we migrate to something superior to layers anyway - no point making the change harder than it already is.
[14:50] <jtv> something I don't understand about your atexit handler: how does its default argument get handled?  How does it avoid dereferencing MemcachedLayer._memcached_process if that's already been torn down?
[14:50] <jtv> review meeting in 10...
[14:52] <stub> jtv: The default value for the pid argument is evaluated when the function is defined.
[14:52] <stub> (Not when the function is invoked)
[14:52] <jtv> stub: Ah.  Also, since the atexit handler is one of those things you won't want to test, can you share a bit more of the killing code between the two places where it's written?
[14:54] <stub> What would you share?
[14:54] <stub> I'd like the tearDown to raise exceptions when things screw up. The atexit is just a second chance of not leaving random processes running if things go wrong.
[14:56] <jtv> hmmm... yeah, not much to share
[14:57] <jtv> only "if pid is not None: os.kill(pid, signal.SIGKILL)"
[14:58] <jtv> It would make tearDown a bit nicer to read though, since you'd be rid of the line break in the "if" condition.
[14:59] <stub> I personally prefer that than the extra indirection
[15:00] <jtv> ok, no particularly strong feelings.
[15:01] <jtv> stub: oh, it's not in the pastebin but above Memcachedlayer.launch you had a double blank line.
[15:02] <jtv> Whereas one is missing above MemcacheClientTestCase
[15:03] <jtv> stub: are you in the reviewers meeting?
[15:03] <stub> Which file is that?
[15:03] <stub> nah
[15:03] <jtv> lib/lp/services/memcache/tests/test_memcache_client.py
[15:04] <stub> fixed
[15:05] <jtv> stub: ok, then r=me
[15:05] <stub> Ta. Now if I can work out how to get lp-dev-deps rebuilt I might one day be able to land it ;)
[15:07] <jtv> I _would_ say "let the Foundations people worry about that," but...
[15:11] <allenap> stub: In line 194 of http://paste.ubuntu.com/282095/ it should be is *not* None I think.
[15:23] <stub> allenap: yup. fixing.
[15:24] <allenap> stub: Cool. I commented on the bug too, just in case you'd gone, not because I like to harass you ;)
[15:26] <jtv> allenap: I've got a pretty urgent MP myself... shall I take it?  Or chuck it on the queue?  :-)
[15:26] <jtv> oh, EdwinGrubbs maybe?
[15:27] <allenap> jtv: I'm doing noodles' review now, but I can push yours on the stack.
[15:27] <EdwinGrubbs> jtv: which review is it?
[15:27] <jtv> allenap: stack?  Not queue?
[15:27] <jtv> EdwinGrubbs: redo-uploads
[15:27] <allenap> jtv: I'm a stack based machine.
[15:27] <jtv> uh-oh
[15:28] <allenap> jtv: Don't worry. I rarely overflow.
[15:28] <jtv> tmi dude, tmi
[15:29] <jtv> EdwinGrubbs: this one: https://code.edge.launchpad.net/~jtv/launchpad/redo-uploads/+merge/12659
[15:29] <EdwinGrubbs> jtv: ok, I'll do it.
[15:29] <jtv> EdwinGrubbs: thanks!
[15:31] <jtv> topic serialization failure
[15:33] <jtv> deryck: what if a bug summary is a full sentence?
[15:33] <jtv> deryck: I sometimes paste an entire error message into a bug search, for instance.
[15:35] <allenap> noodles775: In bug 436182, it says "there's a superfluous newline after libuno-cli-cppuhelper1.0-cil", but it's actually after the comma that follows libuno-cli-cppuhelper1.0-cil. Could that mean that it's really a leading newline in libuno-cli-ure1.0-cil?
[15:35] <mup> Bug #436182: Newlines in Sources.bz2 indices <oem-services> <soyuz-upload> <Soyuz:Triaged by michael.nelson> <https://launchpad.net/bugs/436182>
[15:35] <jtv> deryck: Is "ValueError: The date you gave was in a timezone that is not divisible by 12 minutes, which somehow matters to some hypohetical piece of code." one of these?
[15:36] <noodles775> allenap: that seems to be just the cause of the issue - what happens then is that we (soyuz) incorrectly parse it and add a newline to the end of the field (dsc_binary in this case), as demo'd in the test.
[15:38] <noodles775> allenap: cprov did a separate query showing that the relevant SourcePackageReleases have a '\n' appended to the end of the dsc_binaries field. We should add that to the bug.
[15:45] <deryck> jtv, I think that's ok.  That's why I used quotes.  It's hard to echo on the page nicely, and beuno did sign off on the layout, maybe he has ideas.
[15:47] <beuno> deryck, maybe "Is this the same as: $summary"
[15:47] <beuno> and no quotes?
[15:47] <deryck> beuno, but what is "this"?
[15:47] <allenap> noodles775: This change will affect every field in the index stanza. That sounds like a good bit of belt-and-braces engineering. Might even be worth .strip()ing, not just .rstrip()ing.
[15:47] <beuno> deryck, "Is this bug the same as: $summary?"
[15:48] <noodles775> allenap: I initially had it just on the dsc_binaries, but then chatted with cprov and we decided to apply it generally to all the fields.
[15:48] <deryck> beuno, but it would be more accurately, "Is one of these bugs the same as:  $summary"  which is long, I think.  but I'm not picky.
[15:48] <noodles775> allenap: we've only encountered the problem on dsc_binaries, but it's the same parsing function that parses all the fields.
[15:49] <deryck> beuno, your needed in our room at 4.
[15:49]  * deryck has to have coke before the next meeting, sorry
[15:49] <cprov> allenap, noodles775: and debian policy states it clearly ... http://www.debian.org/doc/debian-policy/ch-controlfields.html
[15:49] <noodles775> allenap: and it's important that it's only rstrip() (imo), as there can be other single line-breaks within the fields values, just not at the end.
[15:49] <cprov> Many fields' values may span several lines;''
[15:50] <beuno> deryck, I will be there in 5ish
[15:51] <jtv> deryck: you're good to go
[15:51] <allenap> cprov, noodles775: Cool, thanks. r=me
[15:53] <jtv> allenap, EdwinGrubbs: sneaking out
[15:53] <jtv> I'll still be here for a while, but not looking for new reviews to do.
[15:55] <noodles775> Thanks allenap !
[15:56] <deryck> jtv, thanks!  Sorry I couldn't be more attentive to review.
[15:57] <jtv> deryck: you're sprinting, no worries.  FWIW I did check that the text gets HTML-escaped, and it does.  Just in case.  :)
[15:57] <deryck> jtv, thanks! :)
[16:47] <EdwinGrubbs> jtv: I don't understand where I go to 'pick a source package in "main" for some Ubuntu release'.
[16:47] <jtv> EdwinGrubbs: https://translations.launchpad.dev/ubuntu/hoary/+source/evolution should do it
[16:48] <jtv> for a dev system
[16:58] <EdwinGrubbs> jtv: When I run reupload-translations.py -s hoary -p evolution
[16:58] <EdwinGrubbs> jtv: I get this: WARNING Found no translations upload for evolution in Ubuntu Hoary.
[16:58] <jtv> EdwinGrubbs: that's normal
[16:59] <jtv> it seems none of these packages have translations uploads in the sample data.  :/
[17:11] <allenap> leonardr: Is this proposal - https://code.edge.launchpad.net/~leonardr/launchpadlib/uses-restfulclient/+merge/5348 - still in need of review?
[17:12] <leonardr> allenap, no, it's long merged
[17:12] <allenap> leonardr: Okay, I'll change the status. Thanks.
[17:13] <EdwinGrubbs> jtv: what does this mean in TestReuploadScript? "INFO\s*Processing [^\s]+ in .*\n"
[17:13] <allenap> gmb: You want someone to review ajaxify-branch-linking?
[17:13] <jtv> EdwinGrubbs: a log message that looks like INFO    Processing <something>
[17:14] <jtv> EdwinGrubbs: the package names are arbitrary thanks to the factory, and I don't want to rely on the ordering even if they weren't.
[17:16] <gmb> allenap: No, Tom's already reviewed that; waiting for a child branch to be ready for reviewing, but I've been sidetracked again.
[17:16] <gmb> this is going to be one of those I-only-land-one-branch weeks, I think.
[17:17] <allenap> gmb: Okay, cool.
[17:17] <gmb> allenap: Thanks for reviewing that trac-launchpad-migrator branch. I'd forgotten it even existed.
[17:17] <allenap> intellectronica: Could you mark https://code.edge.launchpad.net/~gmb/launchpad/ajaxify-branch-linking/+merge/12514 as reviewed by you, so it disappears off the "Requested reviews I can do" bit on +activereviews please? :)
[17:17] <gmb> allenap: We need an nag email for merge proposals, I think...
[17:17] <intellectronica> allenap: sure thing
[17:18] <allenap> gmb: Yeah, I noticed it when I started using launchpad-project/+activereviews. Got a bit of a shock!
[17:18] <allenap> gmb: Are you getting sidetracked by checkwatches?
[17:18] <intellectronica> maaan, the edit button still evades me whenever i go to this page :)
[17:18] <gmb> intellectronica: You're not the only one.
[17:18] <gmb> I think it should still be next to the status myself, but maybe that should only happen when status editing is ajaxified.
[17:19] <allenap> intellectronica: Thanks!
[17:19] <adeuring> allenap: EdwinGrubbs: could one of you please review this MP: https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-refactor-haldevice/+merge/12669 ?
[17:21] <allenap> adeuring: I don't think I'll have time today.
[17:21] <adeuring> allenap: np
[17:22] <allenap> sinzui: Are you able to land https://code.edge.launchpad.net/~sinzui/launchpad/sprint-is-physical/+merge/12026 now?
[17:22] <allenap> sinzui: I'm just doing some late late spring cleaning in +activereviews.
[17:25] <allenap> EdwinGrubbs: Are you reviewing https://code.edge.launchpad.net/~jtv/launchpad/bug-436426-2/+merge/12610?
[17:27] <EdwinGrubbs> jtv: Why are only uploads in the ROSETTA_TRANSLATIONS format re-uploaded? What other formats are there?
[17:28] <jtv> allenap: he's doing my other one
[17:28] <EdwinGrubbs> allenap: no, I'm reviewing the redo-uploads branch
[17:28] <jtv> EdwinGrubbs: the other formats are Soyuz stuff unrelated to Translations.  "Format" looks to be a bit of a misnomer IMHO
[17:29] <EdwinGrubbs> jtv: It also seems that there is no test which exercises the for-loop which calls addOrUpdateEntriesFromTarball().
[17:29] <jtv> EdwinGrubbs: it has to do with the way a single source upload spawns various work items making their own respective ways through the Soyuz work queue.
[17:29] <allenap> EdwinGrubbs, jtv: Okay, thanks. I don't actually have time to review it so I'm not sure why I was asking now!
[17:30] <jtv> EdwinGrubbs: you mean, before I exercised it in this branch?
[17:30] <jtv> allenap: that'll teach you  :-P
[17:30] <jtv> EdwinGrubbs: nm, I misread
[17:31] <jtv> EdwinGrubbs: you're talking about _processPackage, right?  That's called from main(), and is exercised in test_processPackage
[17:31] <sinzui> allenap: The branch bounced many times. mthhadon fixed db-devel a few hours ago. (It was stuck in release-critical)
[17:32] <allenap> sinzui: Okay. I'll mark it as approved then so it drops out of the reviews list, if that's okay.
[17:33] <sinzui> oh. thanks
[17:34] <EdwinGrubbs> jtv: ok, so that test would not have output like "WARNING\s*Found no translations upload for" if you were testing the output?
[17:34] <EdwinGrubbs> jtv: so, is this script just going to be run once manually for karmic and probably never run again unless there is another bug?
[17:36] <jtv> EdwinGrubbs: right on both counts
[17:36] <jtv> want me to inject a mock logger and test the output?
[17:37] <jtv> (actually there's easier ways to check: uploadless_packages)
[17:38] <jtv> EdwinGrubbs: oh, I already check that uploadless_packages is empty there.
[17:38] <jtv> Packages are appended to that in tandem with producing that log message.
[17:38] <jtv> But the test needs a comment!
[17:41] <jtv> Done.
[17:42] <EdwinGrubbs> jtv: and I assume that summarize_translations_queue() ensures that the main() actually processed something, so an empty uploadless_packages couldn't be because nothing was processed?
[17:45] <EdwinGrubbs> jtv: merge-conditional
[17:46] <jtv> EdwinGrubbs: it summarizes what's on the translations queue for that package.  Since the package was just created, it was previously empty.
[17:46] <jtv> The test verifies that _processPackages did upload files to the queue.
[17:46] <jtv> So a pretty thorough test; not just the script's own opinion.
[17:47] <jtv> EdwinGrubbs: thanks!
[17:50] <EdwinGrubbs> jtv: did you want me to review your other branch when I get back from lunch? bug-436426-2?
[17:50] <jtv> EdwinGrubbs: if you have time, yes please; but not as urgent.
[21:53] <barry> EdwinGrubbs: ping