| StevenK | rick_h__: Your displayname seems to be 'Rick harding' | 00:09 |
|---|---|---|
| StevenK | In Buildbot, at least | 00:10 |
| rick_h__ | StevenK: hmm, ok. Looking around | 00:11 |
| rick_h__ | ah, there it is. Thanks StevenK | 00:11 |
| jelmer | StevenK: curtis setting a trend perhaps? | 00:11 |
| rick_h__ | I promise to do better next psm submit | 00:12 |
| StevenK | jelmer: That was firstname, not surname. But perhaps. | 00:12 |
| rick_h__ | pqm...ugh | 00:12 |
| StevenK | Haha | 00:12 |
| StevenK | wgrant: So the rejection is caused due to the validator on SPR.{creator,maintainer} -- Perhaps we should reject just before we create the SPR? | 00:23 |
| wgrant | StevenK: Right. | 00:23 |
| StevenK | wgrant: NascentUpload doesn't create the SPR? | 00:23 |
| wgrant | It uses createUploadedSourcePackageRelease | 00:24 |
| StevenK | Indirectly, it looks like. | 00:24 |
| wgrant | Should be direct | 00:25 |
| wgrant | in dscfile, probably. | 00:25 |
| StevenK | Right | 00:26 |
| StevenK | wgrant: You'd prefer dscfile.storeInDatabase() or IDistroSeries.createUploadedSourcePackageRelease() do the rejection? | 00:28 |
| wgrant | StevenK: Depends where the error handling tends to be now. | 00:29 |
| wgrant | It may want to be even before storeInDatabase. | 00:29 |
| wgrant | Where it's mapping the address to a person, perhaps. | 00:29 |
| StevenK | That's parseAddress | 00:30 |
| StevenK | Right, we grab them there. I can swoop in and do the private bit | 00:32 |
| lifeless | bah, hate it when I can't find a bug | 02:21 |
| StevenK | Bleh. How hard can it be to create a private team with a contact address? :-( | 02:28 |
| wgrant | StevenK: factory.makeTeam(email="foo@bar.com", visibility=PersonVisibility.PRIVATE)? | 02:33 |
| StevenK | wgrant: Unauthorized: (<Person at 0xe3ea090 team-name-100003 (Team Name 100003)>, 'setContactAddress', 'launchpad.View') | 02:34 |
| wgrant | StevenK: with celebrity_logged_in('admin') | 02:35 |
| wgrant | Or fix the factory. | 02:35 |
| lifeless | meep | 02:46 |
| lifeless | hth is https://bugs.launchpad.net/launchpad/+bug/1293 | 02:46 |
| _mup_ | Bug #1293: Customizing body attributes seems like a very secretive, if not almost impossible thing to do <lp-foundations> <Launchpad itself:Invalid> < https://launchpad.net/bugs/1293 > | 02:46 |
| lifeless | on the first page of 'by most recent change' bugs in LP | 02:46 |
| lifeless | (with dups shown and all statuses) | 02:47 |
| lifeless | brain melting | 02:57 |
| lifeless | 1112 to go | 03:16 |
| StevenK | wgrant: I'm having a brainfade - does IPerson.inTeam(ITeam) work? | 03:19 |
| StevenK | Seems to | 03:22 |
| wgrant | StevenK: That's the point of it, so yes :) | 03:23 |
| lifeless | StevenK: " | 04:14 |
| lifeless | Forbid private teams to be private teams." | 04:14 |
| StevenK | Bah | 04:14 |
| lifeless | also | 04:14 |
| lifeless | this leaks the teams, do we care? | 04:14 |
| lifeless | or is changed-by gpg authenticated ? | 04:15 |
| StevenK | lifeless: sinzui and I feel it is safe enough. And we aren't expecting this codepath to be hit often at all | 04:16 |
| lifeless | using the uploader would be authenticated | 04:16 |
| lifeless | rather than the changer | 04:16 |
| lifeless | or am I confused about which field is which | 04:16 |
| StevenK | lifeless: I can switch to SignableTagFile.signer if you wish | 04:19 |
| StevenK | That may leak the team, if the changed-by isn't a member of the team and the uploader is | 04:19 |
| lifeless | so, there are two aspects here I think. | 04:20 |
| lifeless | who do we tell | 04:20 |
| lifeless | what do we tell them | 04:20 |
| wgrant | Is that inTeam bit a useful distinction? | 04:20 |
| lifeless | if we tell unauthenticated people, then we shouldn't tell them anything | 04:20 |
| StevenK | Reject silently? | 04:21 |
| wgrant | If someone wants to know that it was a private team, they can just grep for 'Invalid Maintainer' | 04:21 |
| lifeless | if we only tell authenticated people then we can tell them more | 04:21 |
| lifeless | Well | 04:21 |
| lifeless | I think we should accept it but not link to the person object; but thats clearly different. | 04:21 |
| wgrant | And clearly breaks everything. | 04:21 |
| wgrant | Maintainer and Changed-By are mandatory. | 04:21 |
| lifeless | (It is however the logical consequence of allowing user supplied data to be injected into a partially-private system) | 04:21 |
| StevenK | lifeless: We can't. | 04:22 |
| wgrant | And not linking to the person object still reveals the private team's existence. | 04:22 |
| lifeless | arguably yes | 04:22 |
| lifeless | though linking and then folk dropping that address from their account is win too | 04:23 |
| wgrant | Sure, something like BranchRevision is probably better. | 04:23 |
| lifeless | so whats the reason we don't accept this? I mean, why is rejecting it the approach decided on? | 04:23 |
| wgrant | Er | 04:23 |
| wgrant | RevisionAuthor | 04:23 |
| StevenK | lifeless: Because we already do reject, just the message is bong. | 04:23 |
| lifeless | ah. | 04:24 |
| wgrant | And what else can we do? | 04:24 |
| lifeless | so, uhm. I'd be cautious here. | 04:24 |
| lifeless | wgrant: we could accept; permit limitedview to the relevant context, tell the user they did this thing and they should delete the package to hide it | 04:24 |
| wgrant | lifeless: No. | 04:24 |
| lifeless | we do have private archives after all | 04:24 |
| wgrant | We can't do stuff like that. | 04:25 |
| StevenK | lifeless: And then if the publication is copied to a public archive? | 04:25 |
| wgrant | It's the antithesis of the disclosure project. | 04:25 |
| lifeless | StevenK: prompt them before permitting it | 04:25 |
| lifeless | wgrant: I don't follow | 04:25 |
| wgrant | lifeless: We are going to have 100 different unauditable paths to private team visibility. | 04:26 |
| StevenK | lifeless: Which the copy architecture doesn't permit either | 04:26 |
| wgrant | lifeless: When disclosure is about making visibility less opaque. | 04:26 |
| wgrant | And more auditable. | 04:26 |
| StevenK | BAAAAH | 04:27 |
| lifeless | wgrant: that is something we *can* provide; disclosure is certainly about making it understandable and controllable. Social teams are about making private teams more useful. | 04:27 |
| lifeless | so, right here, right now, I would just say invalid maintainer | 04:27 |
| wgrant | lifeless: And your proposed social private team design is not understandable or controllable. | 04:27 |
| lifeless | make folk that want to probe for private teams work a little | 04:27 |
| StevenK | test_sigint_exits_nicely (bzrlib.plugins.lpserve.test_lpserve.TestLPServiceInSubprocess) just failed on buildbot. | 04:27 |
| lifeless | wgrant: its certainly controllable, as proposed. | 04:28 |
| lifeless | wgrant: and it seems to be quite understandable to everyone I've explained it to. | 04:28 |
| wgrant | "Who can see Canonical Supersecret Team: anyone who can see these 1000 bugs, 200 blueprints, 3 projects, 2 milestones, 4 packages, 2 teams, 3 sprints, 1 translationgroup, 10 branches, 9 code review comments, 4 code review votes, 10 bug subscriptions." | 04:30 |
| wgrant | Very understandable :) | 04:30 |
| lifeless | thats like saying 'who can use LP: anyone that can process http, https and handle a number of forms of crypto, has a working RFC2870... | 04:33 |
| lifeless | its bollox to cast it that way, when we have software to deal with abstractions | 04:33 |
| wgrant | That can't be significantly abstracted. | 04:34 |
| wgrant | Apart from collapsing code review comments and votes into merge proposals. | 04:34 |
| lifeless | We will be face to face soon | 04:34 |
| lifeless | we can drill into it as much as you want | 04:34 |
| lifeless | 'interacts with' seems to be pretty understandable, in my unscientific poll | 04:34 |
| lifeless | I *think* you are thinking implementation | 04:35 |
| lifeless | which is totally different (but still important) | 04:35 |
| wgrant | Hmm? | 04:35 |
| wgrant | I manage a team. | 04:35 |
| wgrant | Someone has discovered its existence :( | 04:35 |
| wgrant | I don't know how. | 04:35 |
| lifeless | that was code for 'not now' | 04:35 |
| wgrant | I look at the disclosure page. | 04:35 |
| wgrant | I see 2000 artifacts that might have leaked it. | 04:35 |
| wgrant | I can't see a list of people :( | 04:35 |
| wgrant | I am screwed. | 04:35 |
| lifeless | IMPLEMENTATION | 04:35 |
| wgrant | I disagree, but next week :) | 04:36 |
| lifeless | wgrant: if its just been disclosed, the page should show precisely one thing: the disclosing thing (because public disclosure has to list every user ever otherwise) | 04:36 |
| lifeless | feel free to help me zerg 75% of the high bugs | 04:37 |
| wgrant | disclosed != puiblicly disclosed | 04:37 |
| StevenK | lifeless: So I should just raise UploadError('Invalid Maintainer.') ? | 04:38 |
| lifeless | so, you're talking teams, disclosure is focused on projects. | 04:38 |
| lifeless | StevenK: it avoids any complication about disclosure | 04:38 |
| wgrant | lifeless: Sure, but making private teams infinitely worse than they are now is not a good goal to have. | 04:38 |
| lifeless | StevenK: I *hate* 'this is wrong but we won't hit it much' cases: they always, eventually, bit. | 04:38 |
| lifeless | *bite* | 04:38 |
| lifeless | wgrant: if that was the goal, I would agree. | 04:38 |
| lifeless | wgrant: you are being very over the top though, which isn't all that nice | 04:39 |
| wgrant | I'm not being hugely hyperbolic :) | 04:40 |
| lifeless | enough to impede any discussion | 04:40 |
| StevenK | wgrant: You forced buildbot again? | 04:42 |
| wgrant | I did. | 04:42 |
| StevenK | lifeless: diff updated | 04:45 |
| lifeless | yay timeouts | 04:53 |
| lifeless | how is that even possible | 04:53 |
| lifeless | Time: 29.29 secondsID: 2, status: 200 (Ok) | 04:53 |
| wgrant | Where? | 04:54 |
| lifeless | wgrant: win!https://bugs.launchpad.net/launchpad/+bug/620989 | 05:06 |
| _mup_ | Bug #620989: Package sets can include sets in other series <lp-soyuz> <packagesets> <Launchpad itself:Triaged> < https://launchpad.net/bugs/620989 > | 05:06 |
| wgrant | Yeah | 05:07 |
| lifeless | didn't we have a policy about tech-debt importance ? | 06:02 |
| wgrant | I don't recall. | 06:03 |
| lifeless | nigelb: https://bugs.launchpad.net/launchpad/+bug/5283 | 06:06 |
| _mup_ | Bug #5283: "Home page" vs. "Description" is misleading <bad-commit-11051> <easy> <lp-registry> <qa-ok> <tech-debt> <ui> <Launchpad itself:In Progress by nigelbabu> < https://launchpad.net/bugs/5283 > | 06:06 |
| lifeless | wow, logintoken is crufty | 06:12 |
| lifeless | ahhhha | 06:13 |
| lifeless | look at the 'may be notified' portlet on https://bugs.launchpad.net/launchpad/+bug/352000 | 06:13 |
| _mup_ | Bug #352000: javascript errors are hard to read, not informative, and often ugly <Launchpad itself:Triaged> < https://launchpad.net/bugs/352000 > | 06:13 |
| wgrant | lifeless: What about it? | 06:14 |
| wgrant | It's backwards, but that's been the case for months. | 06:14 |
| lifeless | Find the names begininning with A | 06:14 |
| lifeless | well, I notice had not | 06:14 |
| lifeless | is there a bug ? | 06:14 |
| wgrant | Don't thinkso. | 06:14 |
| wgrant | It happened in the post-release complete redesign of the subscription UI. | 06:15 |
| lifeless | bah, the mine regression bug isn't in the lpbugtags page | 06:16 |
| lifeless | tag isn't in.. | 06:16 |
| * lifeless uses regression | 06:17 | |
| lifeless | bug 912137 | 06:17 |
| _mup_ | Bug #912137: subscribers portlet sorted in reverse order <regression> <Launchpad itself:Triaged> < https://launchpad.net/bugs/912137 > | 06:17 |
| wgrant | Hm | 06:17 |
| wgrant | Bug #911752 is possible relevant | 06:18 |
| _mup_ | Bug #911752: Bug.getIndirectSubscribers() does not sort using person_sort_key <tech-debt> <Launchpad itself:Triaged> < https://launchpad.net/bugs/911752 > | 06:18 |
| wgrant | possibly | 06:18 |
| lifeless | certainly, that will sort it more correctly... in reverse order | 06:18 |
| lifeless | sinzui: I think you fixed https://bugs.launchpad.net/launchpad/+bug/153343 | 06:22 |
| _mup_ | Bug #153343: Split out email interface tests <email> <lp-bugs> <tech-debt> <test-system> <Launchpad itself:Triaged> < https://launchpad.net/bugs/153343 > | 06:22 |
| lifeless | uhm | 08:09 |
| lifeless | where has 'open team' gone ? https://launchpad.net/~maas-developers/+edit | 08:09 |
| lifeless | I bet I know, I bet 'subteams' includes 'this' and its a regression | 08:10 |
| lifeless | nope, not that simple | 08:13 |
| stub | lifeless: Has it been assigned to anything that would block it from being open? | 08:20 |
| lifeless | it has a list | 08:20 |
| lifeless | but lists are allowed to be open | 08:20 |
| lifeless | its not private | 08:20 |
| lifeless | its not a member of anything AFAIK | 08:21 |
| lifeless | https://launchpad.net/~maas-developers/+participation | 08:21 |
| StevenK | lifeless: ITeam. def checkOpenSubscriptionPolicyAllowed(self, policy='open'): | 08:24 |
| StevenK | Bah | 08:25 |
| StevenK | lifeless: If it's a pillar owner, security contact, has a PPA, closed super teams or is subscribed to or assigned private bugs | 08:25 |
| lifeless | no, no, no, no, shouldn't be yet, shouldn't be yet. | 08:26 |
| StevenK | Apparently, the code disagrees. | 08:26 |
| lifeless | it fails to say so | 08:26 |
| lifeless | it just hides the buttons rather than explaining they are hidden or something | 08:27 |
| lifeless | StevenK: you are an owner of maas-developers, give it a go | 08:27 |
| lifeless | no bugs in 'all related bugs' | 08:27 |
| StevenK | lifeless: Er, it owns maas? | 08:29 |
| lifeless | ah | 08:30 |
| lifeless | well it shouldn't, its the damn list | 08:30 |
| StevenK | https://launchpad.net/maas << meet the pillar | 08:30 |
| lifeless | thanks | 08:30 |
| lifeless | so | 08:30 |
| lifeless | uhm | 08:30 |
| wgrant | Is MaaS developers really the list? | 08:31 |
| StevenK | However, please file a bug, tagged disclosure. The page should state why Open and Delegated (in this example) are hidden. | 08:31 |
| wgrant | Or is it confused? | 08:31 |
| StevenK | The maintainer is the team | 08:31 |
| wgrant | Yes. | 08:31 |
| wgrant | But that doesn't say anything about the intent of the team. | 08:31 |
| wgrant | Apart from that it may be confused. | 08:32 |
| StevenK | sinzui went through this. Open teams that own projects can allow people to join and subvert the team | 08:32 |
| StevenK | Er, subvert the project | 08:32 |
| wgrant | I'm not arguing about the LP rules. | 08:33 |
| wgrant | I'm arguing that whoever created the team might not know what the team is meant for. | 08:33 |
| lifeless | julian made the list :P | 08:33 |
| lifeless | yes, its the list. | 08:33 |
| wgrant | Since it seems to be pretending to both be a privilege team and a list team. | 08:33 |
| StevenK | Oh, right. | 08:33 |
| lifeless | I think he forgot to separate. | 08:33 |
| wgrant | Which is wrong. | 08:33 |
| lifeless | so I've proposed to the cabal in question that we do, ditching the current list and shuffling, to follow the normal naming idioms | 08:33 |
| lifeless | StevenK: thanks very much for spotting that. Care to file a bug that its a PITA to spot ? | 08:34 |
| StevenK | lifeless: I asked you to three minutes ago. :-P | 08:34 |
| wgrant | Open teams are, in general, a Bad Idea™ | 08:34 |
| lifeless | StevenK: yes, I'm currnetly review 1K high bugs | 08:34 |
| stub | https://bugs.launchpad.net/launchpad/+bug/905044 has a script that needs to be brought in-tree, but the license is incompatible. I should just stick a new license at the top since we wrote it? | 08:34 |
| _mup_ | Bug #905044: Some script server scripts are not using pgbouncer for DB connections <canonical-losa-lp> <Launchpad itself:Triaged> < https://launchpad.net/bugs/905044 > | 08:34 |
| lifeless | stub: yes | 08:34 |
| lifeless | StevenK: so my brain is pudding; I'd really appreciate you recording the issue | 08:35 |
| nigelb | lifeless: I was stuck on that because of garbo job. | 08:36 |
| nigelb | I intend to find some time to clear up bugs I've assigned to myself. | 08:36 |
| StevenK | lifeless: bug 912159 | 08:36 |
| _mup_ | Bug #912159: ITeam:+edit does not explain why some subscription policies are hidden <disclosure> <Launchpad itself:Triaged> < https://launchpad.net/bugs/912159 > | 08:36 |
| lifeless | thanks | 08:36 |
| StevenK | If you wish to subscribe/do whatever | 08:37 |
| StevenK | nigelb: 329!! | 08:37 |
| lifeless | oh, I am. To /launchpad-project | 08:37 |
| lifeless | nigelb: did you need advice or ? | 08:37 |
| nigelb | lifeless: 329? | 08:38 |
| nigelb | lifeless: I do. On work week + vacation now, will ping when I'm back. | 08:38 |
| lifeless | kk | 08:39 |
| StevenK | nigelb: You haven't seen the cricket? | 08:39 |
| nigelb | StevenK: Nope. 'm on vacation, on a beach. I thought you don't watch cricket? | 08:39 |
| StevenK | I like it when Australia are winning :-P | 08:39 |
| StevenK | And Clarke hit 329 not out today | 08:40 |
| nigelb | ha, so you only troll when australia is winning? :D | 08:40 |
| StevenK | Pretty much, yes. | 08:40 |
| nigelb | OMGWTBBQ 329! | 08:40 |
| nigelb | *and* a 150 from Hussey. | 08:41 |
| nigelb | Shame. | 08:41 |
| adeuring | good morning | 08:47 |
| lifeless | stub: catchup ? | 09:00 |
| stub | lifeless: sure | 09:00 |
| mrevell | Morning | 09:01 |
| nigelb | Hey mrevell | 09:01 |
| === almaisan-away is now known as al-maisan | ||
| jtv | Mommy, where do NascentUploads come from? | 09:29 |
| wgrant | jtv: process-upload.py | 09:38 |
| jtv | wgrant: I suppose it's okay to construct them directly in tests? | 09:38 |
| wgrant | jtv: It's reasonable. | 09:39 |
| wgrant | Check the existing tests to see what is reasonably distasteful, I suppose :/ | 09:39 |
| wgrant | None of them are pretty. | 09:39 |
| jtv | Thanks. | 09:40 |
| bigjools | jtv: what are you needing to test? | 09:53 |
| jtv | bigjools: the scenario where we sync a debian source package into ubuntu, and then the resulting binary build in ubuntu fails. | 09:54 |
| bigjools | I hate these tests, but ... | 09:54 |
| jtv | But? | 09:55 |
| bigjools | lib/lp/soyuz/doc/distroseriesqueue-notify.txt | 09:55 |
| bigjools | you are checking emails IIRC? | 09:55 |
| jtv | Yes. | 09:56 |
| bigjools | jtv: you might not need to create a NascentUpload, that's all | 09:57 |
| jtv | I figure it's probably simpler than reverse-engineering a suitable path through process-upload. | 09:57 |
| bigjools | NU is quite heavyweight | 09:57 |
| bigjools | the uploader is so badly written that it's very hard to write short unit tests | 09:58 |
| lifeless | flacoste is going to crucify me :) | 10:12 |
| jtv | wgrant: any idea why do_reject might be giving me a ForbiddenAttribute on (createQueueEntry, PackagePublishingPocket.RELEASE)? | 10:17 |
| wgrant | jtv: w... what? | 10:17 |
| wgrant | lifeless: Oh? | 10:17 |
| jtv | wgrant: I guess that's a pocket sneaking into the spot that's reserved for a permission? | 10:18 |
| lifeless | wgrant: I just found 3? 4? hidden criticals | 10:18 |
| wgrant | That looks more like you're trying to call pocket.createQueueEntry, I think | 10:18 |
| lifeless | oopses triaged high | 10:18 |
| wgrant | lifeless: Ah, yeah | 10:18 |
| jtv | wgrant: it's distroseries.createQueueEntry, in do_reject. | 10:19 |
| wgrant | jtv: You've probably got the distroseries and pocket args around the wrong way | 10:19 |
| jtv | Oooh that might be worth checking, thanks | 10:19 |
| bigjools | jtv: sounds also like you are testing in the wrong layer | 10:21 |
| jtv | I'm using LaunchpadZopelessLayer; open to suggestions. | 10:21 |
| wgrant | That's fine. It's ForbiddenAttribute, not Unauthorized. | 10:21 |
| wgrant | (you get Unauthorized in Functional vs Zopeless cases) | 10:22 |
| jtv | Found it. Silly typo. | 10:24 |
| wgrant | Phew. | 10:24 |
| jtv | Don't worry _too_ much — I'm trying to write a test here, not trying to upload a security fix to Ubuntu. :) | 10:27 |
| cjwatson | Do garbo jobs need to land anywhere special (db-devel?) or do they just land on devel? Any special deployment runes related to them? | 10:51 |
| lifeless | devel. | 10:51 |
| lifeless | none. | 10:51 |
| cjwatson | rah | 10:51 |
| cjwatson | Now all I need is to have some clue how to set chunk sizes | 10:52 |
| lifeless | you don't | 10:52 |
| cjwatson | maximum_chunk_size at least | 10:52 |
| lifeless | shouldn't need to touch that at all, it will start at one and scale until the transactions are a good length. | 10:52 |
| stub | Unless the operations are really variable, which can break the tuning :) | 10:53 |
| cjwatson | Oh. All the existing garbo jobs set it though | 10:53 |
| lifeless | should really clean that up | 10:53 |
| cjwatson | And the docs say " maximum_chunk_size = None # Override. | 10:53 |
| cjwatson | " | 10:53 |
| cjwatson | well, insofar as that's doc | 10:53 |
| lifeless | yeah, should cleanup | 10:53 |
| lifeless | set it to 1000 or something | 10:53 |
| cjwatson | OK | 10:53 |
| lifeless | stub: a smoothed average should accomodate pretty noisy durations | 10:53 |
| cjwatson | Anything else I need to pre-imp chat about wrt bug 911943? | 10:53 |
| _mup_ | Bug #911943: gina imports SourcePackageRelease.dsc_binaries incorrectly <Launchpad itself:Triaged> < https://launchpad.net/bugs/911943 > | 10:53 |
| lifeless | stub: quadratic mean I mean, I think. | 10:54 |
| cjwatson | The bug itself is trivial, it's the cleanup ... | 10:54 |
| stub | Its a safety net. If you are trashing stuff from a single table, I'd jump straight to 10000 unless the delete cascades... | 10:54 |
| cjwatson | I'm actually doing updates in this case | 10:54 |
| stub | lifeless: I'm not sure what jtv's algorithm counts as | 10:54 |
| stub | cjwatson: Something lowish then - 1000 should be fine. | 10:54 |
| cjwatson | I need to change everything that says "foo bar baz" to "foo, bar, baz" - should I be using postgres regexp_replace, or doing it in Python? | 10:55 |
| lifeless | whatever your heart desires | 10:55 |
| stub | cjwatson: PostgreSQL's will be optimal as you just issue an UPDATE statement rather than a SELECT followed by an UPDATE | 10:55 |
| cjwatson | okie | 10:56 |
| lifeless | won't really matter though | 10:56 |
| stub | But PG's regexps are special... check the docs | 10:56 |
| lifeless | because a select + update would not start taking locks until the updates started flowing | 10:56 |
| lifeless | 6/1 1/2 dozen | 10:56 |
| cjwatson | "Binary: libmpdclient-dev (>= 2.0.0~git20090929.fc3bae2-0ubuntu1~ripps2~hardy), libmpdclient2" | 11:15 |
| cjwatson | *headdesk* | 11:15 |
| lifeless | cjwatson: is https://bugs.launchpad.net/launchpad/+bug/669404 still high ? | 11:17 |
| _mup_ | Bug #669404: support linux-any and similar expansions in Packages-arch-specific <lp-soyuz> <soyuz-build> <soyuz-core> <Launchpad itself:Triaged> < https://launchpad.net/bugs/669404 > | 11:17 |
| lifeless | cjwatson: if it hasn't triggered in two cycles... | 11:17 |
| cjwatson | lifeless: Probably not; P-a-s has generally been being cut down | 11:17 |
| cjwatson | lifeless: I could just fix it and make it moot, I suppose :) | 11:18 |
| lifeless | cjwatson: you could; I'm going to triage it down to low anyhow. | 11:19 |
| * cjwatson nods | 11:20 | |
| === al-maisan is now known as almaisan-away | ||
| jtv | bigjools, cjwatson: question about the bug I'm looking at. If we sync a package from debian into ubuntu, triggering a build, and the build fails… who should get notified at all? Definitely not the Debian maintainer, who I gather is also probably also the signer and the author of the last change. Anyone else? | 11:56 |
| jtv | bigjools: and by the way, my test is here: http://bazaar.launchpad.net/~jtv/launchpad/bug-876594/revision/14636 | 11:56 |
| wgrant | It should only be the person who requested the sync. | 11:56 |
| wgrant | I can't see it making sense to email anybody else related to the package. | 11:57 |
| jtv | The person who requested the sync is not currently notified AFAICS. | 11:57 |
| wgrant | Correct. | 11:58 |
| wgrant | Until recently they were not stored anywhere. | 11:58 |
| wgrant | And even now they're barely stored. | 11:58 |
| wgrant | At present there is probably nobody that you can sensible notify. | 11:58 |
| wgrant | sensibly | 11:58 |
| jtv | Don't tell me I have to look up the job. | 11:58 |
| wgrant | You can potentially look at SPPH.creator, except when you can't. | 11:59 |
| jtv | It's not that I mind not notifying anyone per se, it's just that it gets hard to ensure a durably meaningful test if I can only verify that somebody does not get notified. It'd be much more robust if I could show "X gets notified but Y does not." | 11:59 |
| jtv | When can't I look at SPPH.creator? | 12:00 |
| jtv | Oh argh, and who's to say I can find the SPPH? | 12:00 |
| * jtv rummages | 12:00 | |
| wgrant | Right, there's not always an SPPH :) | 12:02 |
| wgrant | But there should be at least one normally. | 12:02 |
| jtv | The unwanted recipient is injected in Upload.notify, and I don't think that has a corresponding SPPH handy. | 12:02 |
| wgrant | Certainly not at present, no. | 12:03 |
| wgrant | Often because there's no publication to notify about. | 12:03 |
| wgrant | eg. if the upload is NEW | 12:03 |
| wgrant | or UNAPPROVED | 12:03 |
| jtv | Actually that may just undermine our whole solution to this bug. | 12:05 |
| jtv | We need the SPR's archive, and the archive that the build was to go into, and compare the two. | 12:06 |
| jtv | Can we get those? | 12:06 |
| jtv | There's an Upload.archive, and an Archive.sourcepackagerelease.archive. | 12:07 |
| wgrant | I'm not sure that's the right thing to do. | 12:08 |
| jtv | I take it the two would indeed differ if the upload was for the binary build of a copied SPR? | 12:08 |
| wgrant | For example, say I am a clueless user and upload a Debian package verbatim to my PPA. | 12:08 |
| wgrant | It will have the Debian maintainer in Changed-By. | 12:08 |
| wgrant | This is currently worked around by suppressing Changed-By notifications for PPAs. | 12:08 |
| wgrant | But that's pretty terrible. | 12:09 |
| jtv | In this particular problem domain, I would argue that with an error margin of ±3 or so, you are the only non-clueless user. | 12:09 |
| wgrant | Hah. | 12:09 |
| jtv | I can see how the case you present here is related, but I don't see how the proposed change would affect it. | 12:09 |
| jtv | If you upload a Debian package to your PPA, don't you get your own SPR? | 12:10 |
| wgrant | You get your own SPR, yes. | 12:10 |
| wgrant | And the archive of the SPR and build will match. | 12:10 |
| jtv | But the current PPA rule will apply. | 12:11 |
| wgrant | Should it? | 12:11 |
| jtv | Without that, I think, the maintainer would only be emailed if the upload were unsigned. | 12:11 |
| wgrant | It's from back in the days when Ubuntu was the only other thing that existed. | 12:11 |
| wgrant | The idea probably has merit for everything. | 12:12 |
| jtv | Certainly sounds like it. I tried to word it that way: uploads the maintainer is not responsible for. | 12:13 |
| jtv | In what situation might there be no signer? | 12:13 |
| jtv | Ahhh no, there's a bit of nastiness. | 12:14 |
| wgrant | assuming that "signer" includes "sync requester"... probably just autosyncs | 12:14 |
| wgrant | I would suggest just ignoring Changed-By and only ever using the signer. | 12:14 |
| jtv | Why would the signer include the sync requester? | 12:14 |
| wgrant | But that doesn't work for sponsored uploads. | 12:14 |
| === allenap changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: allenap | Firefighting: - | Critical bugtasks: 3*10^2 | ||
| wgrant | Signer and sync requester serve the same role. | 12:14 |
| wgrant | They are the person to blame for putting the package in its current location. | 12:14 |
| jtv | The signer isn't the person who signed the last change? | 12:16 |
| bigjools | jtv: if there's a job creator or signer, notify them (in that order). If neither, don't notify (although I can't think why we'd have neither). | 12:16 |
| bigjools | signer is the uploader | 12:16 |
| bigjools | changed-by is the person who signed the dsc, generally | 12:16 |
| wgrant | Except in the case of sponsorship | 12:16 |
| wgrant | Which is not uncommon | 12:16 |
| bigjools | right | 12:16 |
| bigjools | well, sponsorship is the uploader being different from the changed-by | 12:17 |
| Laney | I thought sync sponsors weren't expressed in the SPPH yet | 12:17 |
| jtv | We don't even necessarily have the SPPH in this case, apparently. | 12:18 |
| Laney | you can have a build failure without a SPPH? | 12:18 |
| jtv | If I understand wgrant correctly. | 12:18 |
| wgrant | It's unlikely but possible to not have a current source publication. | 12:18 |
| wgrant | And there may be multiple current source publications. | 12:18 |
| jtv | What about SPR? | 12:18 |
| wgrant | There's always an SPR. | 12:19 |
| wgrant | But that's not useful. | 12:19 |
| bigjools | if the build finishes before the SPPH is created | 12:19 |
| bigjools | best place to look is the job | 12:19 |
| wgrant | The build isn't created until after the SPPH. | 12:19 |
| wgrant | But the SPPH can go away. | 12:19 |
| bigjools | yeah I meant PUBLISHED, thinko | 12:20 |
| bigjools | why can it go away? | 12:20 |
| wgrant | If the package is superseded or deleted. | 12:20 |
| wgrant | Although currently in that case we just crash. | 12:21 |
| jtv | #$%@ why do they have a guard in front of the building to tell me that my bike was still there around 6:30 and probably not around 7:30? | 12:21 |
| wgrant | When trying to accept the build. | 12:21 |
| jtv | Well this is for the rejection case anyway. | 12:21 |
| jtv | Also, this being a binary build triggered by a source sync, will I be able to find a job? | 12:22 |
| bigjools | it doesn't go away, it changes state | 12:22 |
| jtv | Will I be able to find an exact match? | 12:23 |
| === almaisan-away is now known as al-maisan | ||
| jtv | Also, will I upset anyone if I redirect the notification stream at different users? | 12:24 |
| bigjools | I doubt it | 12:24 |
| bigjools | assuming you're talking about build failures | 12:24 |
| bigjools | I can't imagine why you can't get hold of the SPPH for the build and grab .creator | 12:25 |
| bigjools | if that's None then you need to find the signer on the source upload | 12:25 |
| StevenK | Bah, buildbot failed again. | 12:26 |
| jtv | It doesn't look like this code knows for sure that it's dealing with a build failure, actually. | 12:27 |
| StevenK | jtv: notify()? | 12:27 |
| jtv | Although… it's "action" must come from somewhere, right? | 12:28 |
| jtv | self.status | 12:28 |
| jtv | So we do know whether it's a failure. | 12:28 |
| jtv | How would I find the SPPH? | 12:30 |
| bigjools | jtv: look it up given the context on the build (archive, distroseries, source package, version) | 12:34 |
| jtv | And then take the latest of the matches, I guess. | 12:35 |
| bigjools | getPublishedSources() | 12:35 |
| jtv | This sounds like a change that we'd want to make for all statuses, not just failures. Is that right? | 12:36 |
| cjwatson | jtv: I agree with wgrant; it should only be the person who requested the sync. There's a bug about them not being notified. | 12:37 |
| jtv | Not the signer? | 12:37 |
| cjwatson | The signer's the Debian maintainer, right? Not them. | 12:37 |
| jtv | That's what I thought, but now I'm not sure. | 12:38 |
| cjwatson | Imagine you're a Debian maintainer; you do not want to be notified about build failures of your package in each of the 17000 Debian derivatives. | 12:38 |
| jtv | That's the bug I'm trying to solve now. But what I was referring to was the part where the signer was supposed to be the Debian maintainer. | 12:39 |
| cjwatson | Well, we don't re-sign syncs | 12:39 |
| bigjools | no, the signer is not the maintainer | 12:39 |
| bigjools | in soyuz-speak | 12:39 |
| bigjools | the signer is the uploader | 12:39 |
| cjwatson | OK, but either way | 12:39 |
| bigjools | we don't care who signs the dsc | 12:39 |
| cjwatson | (yes, you're right, but) | 12:40 |
| cjwatson | If I upload a package to Debian, I don't want to hear about build failures from derivatives, only from Debian. | 12:40 |
| bigjools | jtv: and no, this should apply just to failures | 12:40 |
| jtv | cjwatson: that's where we started. | 12:40 |
| jtv | That's exactly what we're trying to solve here, which is difficult enough, but since we're in a hurry to stop the outflow of those messages we're bikeshedding a new notification regime into my last-minute branch. :) | 12:41 |
| jtv | Come to think of it, maybe we shouldn't do that. | 12:41 |
| jtv | But there are two reasons for adding these notifications. Hold on to your seat; this may scare you. | 12:43 |
| jtv | One is that it protects my test from quietly becoming meaningless: "X is notified but Y is not" is less susceptible to that than "Y is not notified." | 12:44 |
| jtv | The other is that the maintainer specifically gets added to the recipients list if no blamer (in this case, the signer) is given. | 12:44 |
| bigjools | we don't need a new notification regime - build failure notifications are a separate code path to the upload ones | 12:44 |
| jtv | Oh. No, I got confused again. The maintainer gets added if a blamer _is_ given. Shoot me. | 12:44 |
| jtv | Well this is Upload.notify calling notify, is it not? | 12:45 |
| bigjools | it depends on whether it's an upload failure or a build failure | 12:45 |
| jtv | Looks to my untrained eyes like the problem case could be either. | 12:47 |
| bigjools | indeed - since we separated out uploading from the buildd-manager | 12:47 |
| bigjools | which I suspect is the cause of this bug | 12:48 |
| bigjools | so it might be worth checking the BFN code | 12:48 |
| bigjools | and see how it compares to the failure-to-upload notification code | 12:48 |
| jtv | BFN... build failure notification? | 12:50 |
| bigjools | yes | 12:50 |
| bigjools | soyuz has a lot ofg acronyms :) | 12:50 |
| bigjools | https://dev.launchpad.net/Soyuz/Glossary | 12:50 |
| bigjools | needs adding | 12:50 |
| jtv | OFG, ironically, stands for Opportunity For Growth | 12:50 |
| Laney | yeah, really /no/ mails should get sent to Debian about anything which happens in Ubuntu, and they should all be sent to the person responsible for the upload — failures to upload and so on too, not just build failures | 12:50 |
| bigjools | source uploads are fine, it's just the build uploads | 12:51 |
| bigjools | thankfully build upload failures are rarer | 12:51 |
| jtv | Where is this BFN code you speak of? | 12:51 |
| bigjools | one sec | 12:51 |
| Laney | indeed, was just suggesting being more general if possible | 12:52 |
| bigjools | jtv: lib/lp/soyuz/model/binarypackagebuild.py, see notify() | 12:52 |
| bigjools | Laney: I would love it if that were easy in the existing code :) | 12:52 |
| Laney | :-) | 12:52 |
| jtv | if package_was_not_copied and config.builddmaster.notify_owner: | 12:54 |
| jtv | if (self.archive.is_ppa and creator.inTeam(self.archive.owner) | 12:54 |
| jtv | or | 12:54 |
| jtv | not self.archive.is_ppa): | 12:54 |
| cjwatson | I thought build upload failures were exactly what this bug got filed about | 12:54 |
| cjwatson | It certainly came up on debian-devel and I had to do some rapid preemptive damage limitation | 12:54 |
| bigjools | cjwatson: it is | 12:54 |
| cjwatson | http://lists.debian.org/debian-devel/2011/10/msg00469.html | 12:55 |
| bigjools | I am just pointing jtv at the BFN code because it works, and the uploader needs to do the same thing | 12:55 |
| jtv | The condition I quited seems to be the only case where the BPB notify code cares about the details; apart from that, in non-PPA case, I think it just notifies the archive owner. | 12:55 |
| jtv | s/quited/quoted/ | 12:55 |
| jtv | package_was_not_copied uses the same logic we proposed. | 12:57 |
| jtv | But if the package was copied, and the archive is not a PPA, does anybody (apart from the archive owner) get notified at all? | 12:57 |
| bigjools | jtv: bloody hell, it's email buildd_admins! | 12:57 |
| bigjools | emailing* | 12:57 |
| bigjools | WTAF | 12:58 |
| jtv | On the bright side: only if the archive is not a PPA. How often does that happen? :) | 12:58 |
| bigjools | ... | 12:59 |
| bigjools | ok so ignore *that* code :) | 12:59 |
| bigjools | but you know what to do now I hope | 12:59 |
| jtv | Get drunk? | 12:59 |
| bigjools | \o/ | 12:59 |
| jtv | Seriously, no, if it's not that then I don't. | 12:59 |
| bigjools | jtv: as I said above, look up the SPPH. If there's no creator on it then look up the PackageUpload.signing_key user | 13:00 |
| jtv | The latter is what's currently used. | 13:01 |
| bigjools | if neither are present, don't email | 13:01 |
| bigjools | hmmm | 13:01 |
| bigjools | sorry | 13:01 |
| bigjools | I can see where the confusion comes from then | 13:01 |
| jtv | There is hope in what you say though: | 13:01 |
| bigjools | wait - signing_key is right | 13:02 |
| jtv | notify() is "clever" enough to ward off any attempt at change through parameter-passing. If there's a case where we can just skip the call, we win. | 13:02 |
| bigjools | it comes from the key used to sign the changes file | 13:02 |
| bigjools | but with syncs, it won't be there | 13:02 |
| jtv | Upload.signing_key.owner is currently the person notified. | 13:02 |
| bigjools | yeah, I suspect it's the changed-by for syncs | 13:03 |
| bigjools | anyway, just fix the sync case | 13:03 |
| bigjools | and all will be well | 13:03 |
| jtv | If there's no signing_key for binary uploads resulting from syncs, then we're probably facing notify() finding an alternate victim. | 13:03 |
| jtv | That would be the changer. | 13:04 |
| bigjools | correct | 13:04 |
| jtv | However, if there _is_ a signing_key, it will target both the changer and the maintainer. | 13:04 |
| bigjools | builds don't have signers at all | 13:04 |
| bigjools | the signing key is only on the source upload, if it was uploaded not synced | 13:05 |
| jtv | And of course notify() will pick on the changer either way. | 13:05 |
| bigjools | I think that all you need to do is detect that this is a sync and use SPPH.creator | 13:05 |
| bigjools | job done | 13:05 |
| jtv | Won't there be a changer? | 13:06 |
| bigjools | ignore everything else | 13:06 |
| bigjools | SPPH.creator for syncs is what you need | 13:06 |
| jtv | Okay, I'll shoot for that then. | 13:06 |
| * jtv twiddles | 13:06 | |
| bigjools | so use IArchive.getPublishedSources() | 13:06 |
| bigjools | unless some other code has easy access to the source's publication | 13:07 |
| bigjools | not looked in there for ages | 13:07 |
| Laney | bigjools: this reminds me, do you have or want a bug for exposing sync sponsors in the spph? | 13:10 |
| bigjools | Laney: if there is not one already but I think you or someone already filed it | 13:11 |
| Laney | I think I thought we were repurposing the old one for that | 13:11 |
| bigjools | we don't usually do that | 13:11 |
| Laney | yes, but I was under the impression it was part of the UI fixes | 13:11 |
| Laney | no worries | 13:12 |
| bigjools | my memory sucks, I can't remember :) | 13:12 |
| bigjools | FTR it won't get looked at for a long time unless it gets escalated | 13:12 |
| Laney | I have that impression of Launchpad bugs, yes :P | 13:13 |
| bigjools | too many bugs, not enough devs :/ | 13:13 |
| Laney | there we go | 13:15 |
| jtv | bigjools: when looking for the SPPH, do I also need to match on component? Of if I get multiple matches, do I prefer one with a matching component? | 13:15 |
| jtv | Grrr it would be nice if my mouse pointer came back at this point. | 13:16 |
| cjwatson | Laney: ... or you could submit a branch to fix it - exposing new fields isn't too hard | 13:17 |
| bigjools | jtv: build._getLatestPublication() does what you want | 13:17 |
| Laney | cjwatson: That has crossed my mind :-) | 13:17 |
| jtv | bigjools: can I pick any build from Upload.builds? | 13:17 |
| bigjools | jtv: yes, they all come from the same source | 13:18 |
| jtv | Are there always builds? | 13:18 |
| bigjools | if you have a build upload, yes :) | 13:18 |
| jtv | And if not... I guess I revert to the old behaviour anyway. | 13:19 |
| cjwatson | Is there a trick for debugging failing garbo jobs? Something is throwing an exception and I just get SilentLaunchpadScriptFailure | 13:21 |
| bac | matsubara: the deployment report seems to be inaccurate | 13:31 |
| bac | matsubara: i have a branch merged as r14634 that hasn't been tagged | 13:32 |
| matsubara | bac, looking | 13:33 |
| jtv | bigjools: how do I attach the build(s) to my NascentUpload? | 13:45 |
| matsubara | bac, it seems to be correct. the last successful buildbot run was on r14626 | 13:46 |
| bac | matsubara: pebcak | 13:47 |
| jtv | bigjools: nm, I found a way. Not one I'm proud of, but... | 13:51 |
| jtv | bigjools: running ec2 now, with fingers crossed. I'm putting the branch up for review, and then I really have to leave. | 14:10 |
| jtv | bigjools: Care to review? https://code.launchpad.net/~jtv/launchpad/bug-876594/+merge/87624 | 14:17 |
| allenap | jam: Do you have some time to look at https://bugs.launchpad.net/launchpad/+bug/912287? | 14:34 |
| _mup_ | Bug #912287: test_sigint_exits_cleanly breaks <spurious-test-failure> <Launchpad itself:Triaged> < https://launchpad.net/bugs/912287 > | 14:34 |
| === jcsackett changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: allenap, jcsackett | Firefighting: - | Critical bugtasks: 3*10^2 | ||
| cjwatson | I could use a review of https://code.launchpad.net/~cjwatson/launchpad/gina-dsc-binaries/+merge/87632 if an OCR feels up to it. | 14:57 |
| cjwatson | Please think hard about the SQL since I'm a novice at that :-) | 14:58 |
| allenap | cjwatson: I'll have a look. | 15:03 |
| === matsubara is now known as matsubara-lunch | ||
| rick_h__ | adeuring: ping, I'm back if you want to run through that bug sometime | 15:39 |
| adeuring | rick_h__: ok, mumble? | 15:39 |
| rick_h__ | sure thing | 15:40 |
| rvba | allenap: jcsackett Could one of you guys have a look at this MP please? https://code.launchpad.net/~rvb/launchpad/builders-timeout-903827-2/+merge/87620 | 15:43 |
| allenap | rvba: I'm doing another review at the mo, but I'll look when I'm done unless jcsackett gets to it first. | 15:44 |
| rvba | Cool. | 15:44 |
| jcsackett | irvba: i can take a look now. | 15:48 |
| jcsackett | er, rvba. :-P | 15:48 |
| rvba | jcsackett: thanks | 15:48 |
| jcsackett | rvba: looks like a good improvement. r=me. | 16:02 |
| rvba | ta jcsackett. | 16:02 |
| adeuring | rick_h__: #https://pastebin.canonical.com/57786/ | 16:14 |
| adeuring | rick_h__: class TestMaloneView in lib/lp/bugs/browser/tests/test_bugs,py | 16:17 |
| rick_h__ | adeuring: ah, thanks. | 16:17 |
| adeuring | rick_h__: regarding the sort bug on the page we were talking about: I noticed that the JS code does not issue any HTTP request when a sort button is clicked. | 16:31 |
| rick_h__ | adeuring: ok cool, yea it might not be binding on that page since it's not loading the normal "bootstrap" code on that view | 16:32 |
| adeuring | rick_h__: ah, first step for a fix already! | 16:32 |
| === matsubara-lunch is now known as matsubara | ||
| === salgado_ is now known as salgado | ||
| === abentley__ is now known as abentley | ||
| deryck | rick_h__, review is done finally. r=me, with some minor formatting changes and one question. | 16:50 |
| === al-maisan is now known as almaisan-away | ||
| rick_h__ | deryck: saw that, thank you much. I'll update that | 16:59 |
| deryck | rick_h__, np! | 17:05 |
| rick_h__ | adeuring: still around? | 17:46 |
| adeuring | rick_h__: yes | 17:46 |
| rick_h__ | adeuring: I'm just not able to get a failing test and wondering if I'm just way off base. | 17:46 |
| adeuring | rick_h__: my guess would be that you did not enable the feature flag. | 17:47 |
| rick_h__ | I did, I missed that at first | 17:47 |
| rick_h__ | and now I get a nice giant json dump in my view.render() when I debug | 17:47 |
| adeuring | rick_h__: flags = {u"bugs.dynamic_bug_listings.enabled": u"true"} | 17:47 |
| adeuring | with FeatureFixture(flags): | 17:47 |
| adeuring | view = create_initialized_view(...) | 17:48 |
| rick_h__ | but what I can't make 100% sure is that my test view matches the url I'm testing for | 17:48 |
| rick_h__ | adeuring: right http://paste.mitechie.com/show/488/ | 17:48 |
| rick_h__ | is what I'm working with to start a test out. The context and the view are the right kind of objects for the traceback (IMaloneApp, SimpleViewClass) | 17:48 |
| rick_h__ | what's the way to check the view for the url? using view.request.getURL() just get's me 127.0.0.1 | 17:49 |
| adeuring | rick_h__: looks good. I think it is not strictly necessary to check the URL, but you can do that with a call of canonical_url() | 17:49 |
| rick_h__ | adeuring: yea, I just wanted to sanity check that my html I'm getting is in fact for the route /bugs/+bugs | 17:50 |
| rick_h__ | and not /bugs/something_not_failing | 17:50 |
| adeuring | rick_h__: ah, you need to call view.render() inside the "with" | 17:50 |
| rick_h__ | oh? all the tests in test_bugtask didn't, and my render() result has the dynamic buglisting in it | 17:51 |
| adeuring | rick_h__: well, try it here, I'd bet that the test will pass when you call view.render() without the feature fla setting | 17:52 |
| adeuring | well, that view.render() will fail without the fix for MaloneAPp | 17:52 |
| rick_h__ | adeuring: yea, that threw the exception ok cool. | 17:53 |
| adeuring | rick_h__: great. The difference to the other tests may be that you can call assertSomething() outside the !with", but you must call the tested changes within the "with" | 17:55 |
| * adeuring has to run to an evening meeting | 17:55 | |
| rick_h__ | adeuring: ok, I gotcha. THanks | 17:55 |
| lifeless | and good morning | 18:17 |
| rick_h__ | party lifeless | 18:17 |
| lifeless | gary_poster: hey; you have some high, assigned and probably stale bugs. Can you confirm they are stale, or whatnot? https://bugs.launchpad.net/launchpad/+bug/578854 https://bugs.launchpad.net/launchpad/+bug/588401 so far | 18:18 |
| _mup_ | Bug #578854: Confusing docs: doc/buildout.txt <lp-foundations> <Launchpad itself:Triaged by gary> < https://launchpad.net/bugs/578854 > | 18:18 |
| === deryck is now known as deryck[lunch] | ||
| lifeless | huh, we still have windmill tests in-tree | 18:27 |
| lifeless | I wonder if they are being run | 18:27 |
| gary_poster | lifeless, will look | 18:49 |
| gary_poster | in a few hours :-) | 18:49 |
| gary_poster | or maybe tomorrow | 18:49 |
| lifeless | sure; they are high, I am doing the review of high bugs :) | 18:50 |
| lifeless | flacoste: call? | 18:59 |
| gary_poster | lifeless, flacoste, btw, I intend to do our call in 1 hour on Google hangouts. I'll invite you | 18:59 |
| gary_poster | then | 18:59 |
| lifeless | cool... I'm robertcollins <> gmail.com, for google plus | 19:00 |
| flacoste | gary_poster: fwiw, i wasn't able to get google hangout to work this morning with mrevell his invite never appeared for me | 19:01 |
| lifeless | usually spelt with a . actually, but google does address collapsing | 19:01 |
| flacoste | gary_poster: i'm francis.lacoste@canonical.com | 19:01 |
| gary_poster | ack flacoste and lifeless, hopefully it will work | 19:02 |
| rick_h__ | abentley: ping, have a sec? | 19:03 |
| abentley | rick_h__: sure;. | 19:03 |
| rick_h__ | abentley: so the JS code on the bugs.lp.net/bugs/+bugs is bombing because there's no content in the LP.cache which seems to get set as part of the LaunchpadView automatically | 19:04 |
| flacoste | lifeless: skype me | 19:04 |
| rick_h__ | abentley: that context is used for the lp.client.load_model stuff to help generate the link and such | 19:04 |
| rick_h__ | abentley: so debating on if there's something I can do in the getCacheJSON to get a context or if this is something that should be hacked around in this particular view to fake the context enough for lp.client? | 19:05 |
| rick_h__ | sorry, /content/context | 19:05 |
| rick_h__ | abentley: ^ | 19:05 |
| abentley | rick_h__: Ah. | 19:06 |
| abentley | So this is a top-level page that you're using to search all bugs in Launchpad? | 19:06 |
| rick_h__ | abentley: right | 19:07 |
| abentley | rick_h__: I don't see it being a quick hack sort of thing. I assumed our bug listings always had a context. I'm not sure what violating that assumption does to it. | 19:10 |
| rick_h__ | abentley: yea, that's where I was heading | 19:11 |
| rick_h__ | abentley: I *think* the only bad side effect is that lp.client for getting the url of the ajax request | 19:11 |
| rick_h__ | abentley: but definitely not willing to bet on it | 19:11 |
| abentley | rick_h__: So what's actually oopsing here? | 19:11 |
| rick_h__ | abentley: well the original oops was that BugsBugTask...View didn't implement macro_search_title | 19:12 |
| rick_h__ | abentley: I've updated that and now that the page doesn't oops, the JS fails to run on any ajax interaction | 19:12 |
| rick_h__ | abentley: so two different issues, oops is fixed, but the page overall still fails | 19:13 |
| abentley | Okay. | 19:13 |
| bac | hi jcsackett, could you look at https://code.launchpad.net/~bac/launchpad/904335-export-tags/+merge/87664 | 19:17 |
| abentley | rick_h__: I think it's valid for context to be null for Y.lp.client.load_model. Does that give you enough? | 19:18 |
| abentley | rick_h__: You'll have to update Y.lp.client.load_model to actually support that. | 19:18 |
| abentley | rick_h__: The thing is, that page actually does have a context. | 19:19 |
| abentley | rick_h__: It's just not one with a WS url. | 19:19 |
| rick_h__ | abentley: ok, I'll run with testing the context as null and see how far I get | 19:20 |
| abentley | rick_h__: So perhaps it's better to just change ListingNavigator.load_model to use a trimmed version of the current URL. | 19:21 |
| rick_h__ | abentley: yea, that's what I wasn't sure if there was a better fix to getCacheJSON itself or not | 19:21 |
| rick_h__ | abentley: ok, will check that out then | 19:22 |
| abentley | rick_h__: I don't think getCacheJSON is the issue. It doesn't provide the context, because there's no sane context to provide. The page's actual context is something we don't provide in the web service, and probably shouldn't provide in the web service. | 19:22 |
| rick_h__ | abentley: ok, that's what I was hoping you'd verify. | 19:23 |
| abentley | rick_h__: cool. | 19:23 |
| === deryck[lunch] is now known as deryck | ||
| deryck | abentley, rick_h__ -- I'm going to work on the open questions now, just so no one works over top of me. | 19:29 |
| deryck | abentley, rick_h__ and FYI, since it might take a while. ;) | 19:29 |
| rick_h__ | deryck: understood, leave some breadcrumbs to get back out of there | 19:36 |
| deryck | heh, seriously. | 19:36 |
| === matsubara is now known as matsubara-afk | ||
| gary_poster | flacoste, lifeless google hangout waiting for you | 20:00 |
| flacoste | gary_poster: lifeless might be a little late, was grabbing food | 20:01 |
| flacoste | gary_poster: where should I see your invite? | 20:01 |
| flacoste | gary_poster: and are you using your canonical.com account? | 20:02 |
| gary_poster | flacoste, was using gmail account. I sent two invitations to you just in case | 20:02 |
| flacoste | hmm | 20:02 |
| gary_poster | flacoste search for me | 20:02 |
| gary_poster | it will say I'm in a hangout | 20:02 |
| gary_poster | click on it | 20:02 |
| flacoste | gary_poster: is it the yellow hangout?% | 20:03 |
| lifeless | I don't see an invite yet | 20:05 |
| lifeless | ahha | 20:06 |
| flacoste | lifeless: search for gary and join the yellow hangout | 20:07 |
| lifeless | yeah plugin is installing | 20:07 |
| flacoste | lifeless: if you have found the invite, please tell me where? | 20:07 |
| lifeless | in the 'notifications' drop down from the black bar at the top of the screen | 20:07 |
| rick_h__ | abentley: working on looking at best tests to add but can you peek at this and see if it's too hacky? https://code.launchpad.net/~rharding/launchpad/oops_912178/+merge/87675 | 20:23 |
| rick_h__ | abentley: doing that change *works* locally | 20:23 |
| abentley | rick_h__: You shouldn't need to omit the +bugs if present. | 20:26 |
| rick_h__ | abentley: ok, but the code that builds the url has that as part of it, sec let me get it | 20:27 |
| rick_h__ | abentley: it's the "view_name" passed into load_model | 20:28 |
| abentley | rick_h__: Right. But the view name is only needed if you have a context. Otherwise, it doesn't really make sense. | 20:28 |
| rick_h__ | abentley: which lp.client.get_view_url uses | 20:28 |
| rick_h__ | abentley: hmm, right, but I'm basically faking a context so that everything elses passes through. So I still use get_view_url to build the query params, the ++model++ and such | 20:29 |
| abentley | rick_h__: Since we're using the URL to get this data some of the time, I recommend using the URL all of the time. | 20:30 |
| abentley | rick_h__: and then you can strip all that out. | 20:30 |
| rick_h__ | abentley: are we the only consumers of lp.client.load_model? | 20:30 |
| abentley | rick_h__: No, I believe there's one other call site. | 20:31 |
| abentley | rick_h__: lib/lp/translations/javascript/sourcepackage_sharing_details.js | 20:31 |
| rick_h__ | abentley: ok, so if I change the load_model to use the url vs building it have to check it out over ther eas well | 20:32 |
| abentley | rick_h__: Yes, or else you can make a new function that is a callee of load_model, and accepts a URL instead of context+view_name | 20:33 |
| lifeless | gary_poster: oh hai? | 20:57 |
| gary_poster | lifeless, yeah, trying to get back. though maybe we should just say.... | 20:57 |
| gary_poster | thank you lifeless flacoste bac benji gmb frankban ! | 20:58 |
| gary_poster | bac, I'll be ready for our call as soon as I get back on | 20:58 |
| lifeless | gary_poster: thank you | 20:58 |
| lifeless | gary_poster: francis says next week, talk about success factors | 20:58 |
| gary_poster | cool, sounds good | 20:58 |
| abentley | lifeless or gary_poster: I have a question about eager loading BugAttachments. | 20:59 |
| lifeless | gmb: You have a number of uhm assigned bugs, wondering if you could unassign or update them ? | 20:59 |
| gmb | lifeless: sure thing. I'll do that before I sod off. | 21:00 |
| lifeless | abentley: I'm about to go on a call, but if you ask, I will try to answer as time permits | 21:00 |
| * gary_poster is going on a call too, and will know less | 21:00 | |
| abentley | lifeless: Okay. In the current codebase, BugComment uses cached data from Message, but also has values from bugtask.bug.attachments_unpopulated inserted. This eager loads the data. | 21:02 |
| abentley | lifeless: I'd like to change BugComment so that it delegates to its Message. I had thought that since bugtask.bug.attachments_unpopulated still runs, it would eager load the data into the Storm cache. | 21:03 |
| abentley | lifeless: but in fact, there are extra queries. | 21:03 |
| lifeless | what are the extra queries ? | 21:05 |
| abentley | lifeless: http://pastebin.ubuntu.com/794224/ | 21:05 |
| abentley | lifeless: They are per-Message BugAttachment lookups. | 21:06 |
| abentley | lifeless: Well, one is. Another is a similar issue with message chunks. | 21:06 |
| bac | thanks for the review jcsackett | 21:08 |
| lifeless | abentley: ah, so IIRC bugcomment is an unusual class, its memory only; holds the calculated index | 21:08 |
| lifeless | abentley: is that right? and now the index is persisted, it can do away | 21:08 |
| lifeless | ? | 21:08 |
| lifeless | mmm, possibly not the class I'm thinking of | 21:08 |
| abentley | lifeless: The first part, that it's memory-only, is substantially true. | 21:09 |
| lifeless | abentley: anyhow, you should have a backtrace of the thing triggering the extra query | 21:09 |
| lifeless | last element of the timeline tuple | 21:09 |
| abentley | lifeless: I'm pretty sure I know what's triggering the extra query. | 21:09 |
| lifeless | abentley: ok; an attribute accesss? | 21:09 |
| abentley | Message.bugattachments. | 21:10 |
| lifeless | ok | 21:10 |
| lifeless | so sqlmultiplejoins will always do queries | 21:11 |
| lifeless | they are something we need to purge from the codebase, or at least stop using them | 21:11 |
| lifeless | the workaround is to create a @cachedproperty property that can return the bugattachments, and inject the result into the messages from a decoratedresultset | 21:12 |
| lifeless | you can replace the attributename - e.g. | 21:12 |
| lifeless | _bugattachments = SQLMultipleJoin(...) | 21:12 |
| lifeless | @cachedproperty | 21:12 |
| lifeless | def bugattachments(... | 21:12 |
| lifeless | or you can add a new one and expose in the interface etc etc | 21:12 |
| lifeless | you'll then need to make sure the code path instantiating the objects still does the queries you want and so forth | 21:13 |
| abentley | lifeless: So to make that work here, I think I'd need to inject the values into the cache in the code that uses bugtask.bug.attachments_unpopulated | 21:14 |
| lifeless | sounds plausible yes | 21:14 |
| abentley | lifeless: The cachedproperty definition would be something like "return list(self._bugattachments)" ? | 21:18 |
| lifeless | abentley: extactly | 21:38 |
| abentley | lifeless: How does this look? http://pastebin.ubuntu.com/794262/ | 21:39 |
| === jcsackett|afk is now known as jcsackett_ | ||
| flacoste | gary_poster, lifeless: https://dev.launchpad.net/Projects/ParallelTesting | 22:05 |
| gary_poster | thank you flacoste | 22:06 |
| lifeless | abentley: looking | 22:12 |
| lifeless | abentley: yes, thats the sort of thing | 22:13 |
| abentley | lifeless: thanks. | 22:14 |
| === jcsackett_ changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugtasks: 3*10^2 | ||
| wgrant | Hmm | 22:46 |
| wgrant | I think bug #909318 may be bad | 22:47 |
| wgrant | Half the sort options have gone missing on qastaging | 22:47 |
| wgrant | Oh, nevermind. | 22:48 |
| wgrant | Only the shown fields have sort buttons. | 22:48 |
| wgrant | StevenK: Is your private maintainer QA imminent, or should we deploy now? | 23:22 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!