[00:04] wgrant: Ha. Hahaha === yofel_ is now known as yofel === slank is now known as slank_away [00:37] StevenK: Going well? [00:40] Still making a list of pages to hit up [00:46] Ah [00:48] Trying to find a projectgroup on qas that uses blueprints [00:49] https://qastaging.launchpad.net/heehee [00:50] ! [00:53] wgrant: So that's Person, Product, ProductSeries, Milestone, Distribution, DistroSeries, and Sprint checked. [00:53] heehee has no blueprints, and something with private blueprints would be nice. [00:53] StevenK: It does, you might just not be able to see them [00:54] prop-auditorclient is in it [00:54] And I added a private blueprint to it last week to check this exact thing [00:54] Yeah, I can't view prop-auditorclient [00:54] I've made it public [00:55] The project group now correctly shows an empty blueprint list for an anonymous user [00:55] So it looks fixed [00:55] I still get the message for me [00:55] Which message? [00:55] "... Register the first blueprint in this project! If you have a proposal for a feature ..." [00:56] That's the empty blueprint list :) [00:56] Not so much a list as a wall of text [00:56] I believe it is the largest wall of text in the entire application, indeed [00:56] (ignoring help popups) [00:57] I still can't see the blueprint for prop-auditorclient, but eh [00:58] Oh, projectgroup didn't check for privacy. Right. [00:58] Now it does, since I made it do so. [00:59] Exactlyu [00:59] i've just shared the project with you [00:59] So you should be able to see the spec [00:59] And I can [00:59] Fantastic [00:59] * StevenK goes to mark bugs as qa-ok [00:59] Did you want a deployment? [00:59] Indeed [00:59] I'm about to disappear out to lunch, so if you could arrange that... [01:00] Also, if it happens while I'm gone, do keep an eye on buildd-manager [01:00] I'll sort it out [02:58] wgrant: http://pastebin.ubuntu.com/1561501/ [03:00] StevenK: Does it work? [03:01] The tests pass [03:01] I fear it doesn't close danilos bug [03:03] StevenK: H, though the privacy filter should be in the recursive query, not the outer one [03:03] eg. if we have A->B->C and I can't see B, I think this will do strange things [03:03] whereas if the filter's in the recursive bit, then I will just see A, not A and C [03:04] wgrant: Right [03:06] And both recursive_dependent_query and recursive_blocked_query are just a string [03:06] Maybe not for long :) [03:06] Yeah [03:06] Otherwise you can use that compilation thingy [03:06] convert_storm_clause_to_string [03:07] wgrant: The inner SELECT from the UNION requires the privacy check? [03:07] I haven't actually looked at the recursive query [03:07] * wgrant looks [03:07] Which is on specificationdependency :-/ [03:08] Yeah, so you'll probably want to adjust the second half of each union to do the filtering. [03:08] I didn't realise it was only over specificationdependency, which makes it a bit awkward [03:08] But it shouldn't be too bad. [03:10] So I have to join back to spec and then feed in get_specification_privacy_clause? [03:18] StevenK: Right. [03:19] Hmmm, on specification on dependency, though [03:19] Hm? [03:19] specificationdependency has specification and dependency, both of which are specification [03:20] Whichever one it returns [03:20] Which will be one for blocked, and the other for deps [03:39] Hm, my query is broken [04:07] wgrant: http://pastebin.ubuntu.com/1561618/ [04:10] * wgrant cries [04:10] wgrant: Hm? [04:11] StevenK: a) It's illegal to construct SQL with %. Legacy code is permitted as long as it's of the form "% sqlvalues(...)", but nothing else. Try using SQL() instead. [04:11] b) Have you verified the constructed SQL? [04:11] What does convert_storm_clause_to_string do when given multiple args? [04:12] It isn't, it's either an [Or()] or a [In()] [04:12] Ah [04:12] Still, examine the constructed SQL [04:14] wgrant: http://pastebin.ubuntu.com/1561630/ [04:15] StevenK: lolwut [04:15] Oh, misread [04:16] StevenK: What doesn't it do? [04:16] Oh? [04:16] There's no actual SELECT there [04:16] Just a WITH RECURSIVE [04:16] Which is not completely helpful [04:18] wgrant: http://pastebin.ubuntu.com/1561633/ [04:19] StevenK: and what's broken about it? [04:19] Other than some possibly missing parens [04:20] It was broken, I fixed it by reordering the FROM [04:20] Oh [04:20] I thought it was still broken :) [04:20] But I can't use %s [04:21] Is this a subtle hint that I should Storm-ify it? [04:21] You should be able to use SQL() [04:22] I don't think our current Storm With expression supports RECURSIVE [04:24] wgrant: The callsites do SQL(), the functions are expected to just return a string [04:24] You might want to rearrange that [04:25] Given the paucity of callsites that should be quite trivial [04:28] wgrant: http://pastebin.ubuntu.com/1561643/ [04:29] StevenK: https://code.launchpad.net/~wgrant/launchpad/bug-1100977/+merge/144432 [04:30] 22if content.find(no_key_message) >= 0: [04:30] Sadface [04:30] StevenK: I think your linewrapping violates the UDHR. [04:31] Which linewrapping? [04:31] + )""", params=(spec.id, convert_storm_clause_to_string( [04:31] + *get_specification_privacy_filter(user)))) [04:33] wgrant: And you didn't complain that I only fixed one of the methods? Disgraceful. [04:33] wgrant: http://pastebin.ubuntu.com/1561650/ [04:33] I give up reading un-highlighted diffs easily :) [04:33] (--syntax=diff is your friend) [04:34] That's still a criminal offence in most jurisdictions, but less so [04:35] wgrant: params=(\nspec.id, ... )? [04:36] StevenK: Right :) [04:37] wgrant: http://pastebin.ubuntu.com/1561657/ [04:40] StevenK: Actually, I'm surprised that works. [04:40] Given you're passing a plain string into params [04:40] But trying to use it as SQL [04:40] It may work if you wrap it in SQL(), not sure [04:41] I've not actually tried it ... [04:41] Heh [04:42] You might actually be able to properly Stormify it with a bit of a hack [04:42] With('RECURSIVE dependencies(id)', Union(...)) [04:43] DataError: invalid input syntax for type boolean: "Specification.information_type IN (1, 2)" [04:43] LINE 8: WHERE sd.specification = d.id AND (E'Specificati... [04:43] Hahaha [04:46] Hmmm [04:46] ProgrammingError: can't adapt type 'In' [05:20] wgrant: I'm not sure about the Storm-ified version ... [05:20] Or indeed if I've done it correctly [05:25] wgrant: http://pastebin.ubuntu.com/1561722/ [05:26] StevenK: The easiest way to be sure about it and that you've done it correctly is to see if the tests work [05:28] Bah [05:28] zeca was tested directly [05:28] Haha [05:28] I was about to point that out [05:29] CompileError: Don't know how to compile type Reference of [05:29] Thanks for being helpful, Storm. [05:29] StevenK: Did you do reference == reference? [05:29] That doesn't work [05:29] id == id [05:30] Or reference == id [05:30] works [05:32] Specification.id == SpecificationDependency.specificationID [05:32] That's the only ==, so I'm not sure which reference it's barfing on [05:33] Is it roughly simialr to the code that you pased? [05:33] + 'RECURSIVE blocked(id)', Union(Select( [05:33] + (SpecificationDependency.specification,), [05:33] That might not work [05:33] You probably have to select the ID [05:34] wgrant: The only change is calling the new function further down [05:35] wgrant: 'RECURSIVE blocked', Select((id,), Union(... ? [05:37] StevenK: That'll try to select the builtin function 'id' [05:37] SpecificationDependency.specificationID [05:38] Which is already in the inner select? [05:39] StevenK: Is that a problem? [05:40] Just ... odd [05:41] Not sure how [05:45] Right, RECURSIVE blocked(id) is working because specificationID rather specification [05:45] Except it doesn't love blocked [05:45] So I get an actual statement that compiles but postgres doesn't love. [05:46] What does postgres complain about? [05:48] wgrant: http://pastebin.ubuntu.com/1561762/ [05:49] StevenK: Ah, you should be able to see the problem there [05:50] Select((spec.id,), Union(... ? [05:50] No [05:50] Look at the first line [05:50] Oh, quoting RECURSIVE is a bad thing [05:50] Yes [05:50] With(SQL("RECURSIVE blocked(id)"), ...) might work [05:50] Not sure [05:52] ProgrammingError: syntax error at or near "(" [05:52] LINE 1: WITH (RECURSIVE blocked(id)) AS (SELECT 13 WHERE (SELECT Spe... [05:52] Let's go with no [05:55] StevenK: Bah, it's too smart. [05:55] Heh [05:55] StevenK: Define your own With expression, I guess. [05:55] I don't think the Storm-ifying is buying us anything. If anything, it's far uglier [05:55] (or extend the one in our Storm branch, I guess, would be better0 [05:56] Certainly [05:56] But it would have been a trivial way to integrate the privacy filter apart from the RECURSIVE issue [05:56] Toss it as a param is giving me a wierd error too [05:56] Well, params is for params [05:57] Oh [05:57] I feel %s calling, but I'll get stabbed [05:58] It seems that avoiding it here is somewhat non-trivial [05:58] So it may be permissible [05:58] But inserting the id like that as the original code did is strictly forbidden [05:58] Even in legacy code [08:52] good morning === almaisan-away is now known as al-maisan [09:00] Good morning adeuring! [09:00] hi jtv! === yofel_ is now known as yofel [10:49] hi guys === jtv1 is now known as jtv === Ursinha_ is now known as Ursinha [14:12] wgrant: I'd like to have a pre-imp chat about bug 1102870 [14:12] <_mup_> Bug #1102870: Copies use naïve ancestry check to calculate previous version for notifications and bug closures < https://launchpad.net/bugs/1102870 > [14:13] I *think* a sensible way to approach this is by moving NascentUpload.getSourceAncestry (and probably getBinaryAncestry too, for symmetry) somewhere more sensible [14:13] However, it uses the deprecated DS.getPublishedSources method [14:14] Rephrasing it in terms of Archive.getPublishedSources isn't too hard, but it means more potato programming since there's then no way to query multiple archives at once [14:15] I kind of feel that the method belongs on DS anyway, since it's "give me the current published version of $package in this distroseries, for some set of pockets" [14:16] But I'm not sure that there isn't a better place - do you have any strong feelings? [14:17] (If I put it on Archive, then I have to do explicit version comparison between the results for each pocket, I think, and the method that does that will still have to live somewhere common - so I don't think putting it on Archive would help) [14:19] It might be worth getting rid of one more use of DS.getPublishedSources in any case; something like http://paste.ubuntu.com/1563165/ === al-maisan is now known as almaisan-away === slank_away is now known as slank === matsubara is now known as matsubara-lunch [15:56] benji, bac, if either of you have time today, could you take a look at https://code.launchpad.net/~gmb/lp2kanban/encode-annotations-with-json/+merge/144372? It takes the description annotations work and uses JSON for encoding rather than hand-rolled key=value pairs. [15:56] (per benji's suggestion) [15:56] gmb: sure (bac is a bit sickly at the moment) [15:57] benji, Thanks. === matsubara-lunch is now known as matsubara === matsubara is now known as matsubara-afk [19:54] dzin: hmm sinzui isn't here [19:54] dzin: maybe we need to go to #launchpad! :-) [19:54] ok :) [19:55] joey: he doesnt do LP stuff any more [19:55] czajkowski: ah ha [23:59] wgrant,StevenK: I'd like to get https://code.launchpad.net/~cjwatson/launchpad/avoid-copy-archive-spam/+merge/144611 reviewed reasonably quickly, to ensure we stop spamming its victim