=== wgrant_ is now known as wgrant [00:10] lifeless: OK, I'll just roll it into the code change corresponding to that db change, then === jtv1 is now known as jtv [02:24] Test failures due to a DB patch can be fixed in the DB branch, or should be landed first? [02:25] land first if at all possible [02:25] as we can't make code changes when the DB patch deploys, we want to be sure things will be ok when it deploys [02:25] (and land on devel of course) [02:26] StevenK: What's the issue? [02:26] wgrant: test_branch_privacy_triggers does manual INSERTs into Branch which blows up when it's NOT NULL. [02:27] When information_type is set NOT NULL, sorry. [02:27] Even if it's just test changes, I'd still do it in devel. [02:28] If it's not just test changes, it has no choice but to go to devel. [02:28] Yes, I'm putting together a branch now [02:37] wgrant: i've done a branch behind a feature flag to revoke bug access on unsubscribe but of course the triggers do that and so the test fails. one option is to have the test remove the trigger. any other ideas? [02:38] wgrant: https://code.launchpad.net/~stevenk/launchpad/information_type-branch-privacy-trigger/+merge/105291 [02:39] wallyworld_: wait, what? Do you mean unsubscribe after bug access is revoked? [02:40] lifeless: no. other way around. we already have a job to unsubscribe after access is revoked [02:40] lifeless: Someone hasn't been reading launchpad-dev :) [02:43] wgrant: thread name ? [02:43] [Launchpad-dev] Next steps for Better Privacy [02:43] wallyworld_: you should be able to unsubscribe and retain access in the new world order, surely ? [02:43] lifeless: This is the Transitional World Order [02:43] lifeless: eventually yes [02:43] Not yet New World Order [02:44] More specifically, this is the Transitional While We Have No UI Design But Need To Sort The Model Out World Order [02:44] lifeless: we initially want to mimic current behaviour [02:44] wgrant: I have no such email [02:45] wtf [02:45] lifeless: When did you unsubscribe from -dev? :-P [02:45] StevenK: probably when I subscribed to a different list; +editemails is error prone [02:45] wgrant: i could also make the trigger look at the feature flag table. i think that's the best option [02:45] yup, there we go [02:46] no wonder it was quiet [02:46] lol [02:46] wallyworld_: Ew [02:46] why ew? [02:46] lifeless: Would you like me to forward it? [02:46] wallyworld_: It's unprecedented and there must be a better way [02:47] I'm reading the list archives [02:47] what is wrong with business logic checking feature flags? [02:47] wallyworld_: It's a second implementation of the feature flag logic. [02:47] shouldn't matter that the business logic is in a trigger [02:47] This time in PL/pgSQL [02:47] wallyworld_: The flag table contains a scope. How are you going to get a trigger to work THAT out? [02:47] the only other way that i can see is to have the test disable the tirgger [02:47] As wgrant says, Ew. [02:48] detail, details [02:48] forgot about the scope [02:48] but i was thinking this would be on or off for all [02:48] Right, scope is the main problem. [02:48] You could just always use 'default'; [02:48] so could ignore scope [02:48] If you implement flag scoping in PL/pgSQL, wgrant may murder you. [02:49] But it's still a bit ew, which is why I'm trying to think of a better way [02:49] so i could just ignore scope is what i was thinking [02:49] ok [02:49] wallyworld_: This is the test for when the flag is disabled? [02:49] so I can see this transition [02:49] I have two thoughts [02:49] firstly, have you considered just doing the final thing, directly. [02:50] lifeless: I was surprised when you didn't reply. [02:50] lifeless: I guess this explains it :) [02:50] wgrant: yes [02:51] lifeless: I guess checking the flag, explicitly in the 'default' scope, might be best. Otherwise removing the trigger later without illegally hacking tests might be awkward. [02:51] wgrant: so, AIUI the remove-grants-on-removed-subscriptions is being written to cope with... [02:51] 'there is no UI for showing who has access but not a subscription' [02:52] so if you wrote that UI today (and define it as crudely as possible - e.g. just show 'everyone with access') [02:53] you wouldn't need a job to remove access when a subscription is removed [02:53] it wouldn't be a job [02:53] but yes [02:53] job task code function helper etc [02:53] :P [02:53] lifeless: Sure. [02:53] i was being a pedant :-P [02:53] wallyworld_: orly ? :) [02:54] lifeless: But the UI would be very duplicatastic with the subscriber list, and we would have to introduce a whole lot of new sharing management widgets on the bug page. [02:54] wgrant: so, you're not doing htat.. why not ? [02:54] None of which we have design for, and all our UI people are off doing other things. [02:54] are you planning on doing that anyway ? [02:54] Mostly MAAS [02:54] Yes. [02:54] we don't have to get the presentation perfect first time around [02:55] We have to have it not be utter shit. [02:55] the key elements of design in this are already committed too - this is just the ramifications [02:55] And it has to not be confusing. [02:55] Because this involves privacy. [02:55] I agree. [02:55] So, we can implement this transitional thing in the next roughly two weeks. [02:55] is there a page showing who can see a given bug, today ? [02:55] We cannot get design tested before then. [02:55] lifeless: The subscriber list. [02:55] lifeless: wgrant: should we move the subscribers portlet off the default bugtask view and replace with 'who has access'? [02:56] that makes more sense to me [02:56] wallyworld_: That's for UI people and testing to answer :) [02:56] since i want to know who can see my stuff [02:56] We'll be lucky to get an hour of UI time this month. [02:56] wallyworld_: we could; some folk want to know who will be notified. Arguably the contents of both portlets should be expandable. [02:56] and till now, who can see = who is subscribed [02:56] one way would be to expand the subscribers portlet [02:57] it currently says 'you get xxx, these people get xxx, these people may get xxx', adding 'these other people are able to view the bug but do not get xxx' [02:57] we would need to use expanders i think if we do that, but could be ok [02:57] would make a lot of sense to me [02:57] it would be totally empty for a bug today. [02:57] lifeless: The transitional phase lets us add a single simple extra bit of UI (the policy grantee list) and a couple of extra model bits, and it lets us sort of the model so the new UI can be implemented easily once it's designed. [02:57] and if its the same portlet, there is ~= no perf overhead. [02:58] s/sort of/sort out/ [02:58] w.r.t. removing the triggers, my suggestion would be: [02:58] - have tests that use a feature flag to indicate whether the triggers exist or not. [02:58] and have a DB patch that removes the triggers and sets the feature flag ob. [02:58] s/tests that use.../code that uses.../ [02:58] That's a possibility. [02:59] a small test fixture to give you no triggers + feature rule, will let you write tests now [02:59] you won't be able to go live except in a foul swoop, so you'd want to tightly limit how much code lived under that flag. [02:59] (OTOH we could rollback relatively easily) [03:00] Heh [03:00] lifeless: you mean the fixturewould disable the triggers in its setup? and enable after? [03:00] Foul swoop? :) [03:00] Sounds reasonable, though. [03:00] wallyworld_: yeah. [03:00] ideally you'd write code that doesn't care about the triggers or not. [03:00] Which is why I am also questioning why you're doing that. [03:00] lifeless: i was thinking the same thing when i initially posed this issue above [03:01] The lack of design-team seems like a poor reason not to use your own judgement [03:01] lifeless: well the code is to replace the trigger eventually [03:01] i think it better that the ui be done so the triggers can be removed [03:01] do something tolerable and quick (show them at the end of the subscribers list), and revisit when design have bandwidth. [03:01] +1 [03:01] lifeless: It's not just showing them. [03:02] lifeless: We have to be able to remove them and possibly add them too... [03:02] wgrant: with a (-) button to remove them. [03:02] That's why showing policy grants is light. [03:02] Because you can't remove them from that page. [03:02] wgrant: that's what the +sharing page is for [03:02] lifeless: Who has permission to remove them? [03:02] or not? [03:02] wallyworld_: You really need to be able to revoke from both ends. [03:02] not initially [03:02] wallyworld_: Particularly since only the project owner can remove people on +sharing [03:02] lets separate out needed and wanted [03:02] This is needed. [03:02] sure? [03:03] why? [03:03] we could just do read only portlet on bugtask page [03:03] We can't block on the CC to remove people from bugs... [03:03] wgrant: wallyworld_ is not suggesting that [03:03] be we can remoe people via the sharing page [03:03] wallyworld_: Only the pillar owner can use +sharing [03:03] ah yeah [03:03] sure, so? [03:03] wgrant: so, you can add (-) with the same rules from removing subscriptions, initially. [03:04] wgrant: that is clearly ok, because it works for subscriptions. [03:04] Perhaps. [03:04] why should anyone besides the pillar owner be able to revoke access? [03:04] I can see danhg and huwshimi coming to strangle us in our sleep, though. [03:04] wgrant: you could broaden it to allow the pillar owner to always remove, to be in line with +sharing. [03:04] regardless of if it's done from bugtask or +sharing [03:04] wallyworld_: I've filed a bug containing user data that's private to me. [03:04] wallyworld_: But I can't prevent people from accessing it, because Launchpad is stupid :( [03:05] wgrant: on what basis? That you did /something/, with communication with them about what you're doing (to give them the opportunity to go nononnonoon if needed), and behind a ff, so that its got no UI impact until you're happy with it? [03:05] ok so we can allow bug owner + pillar owner [03:05] lifeless: Anyway [03:05] wallyworld_: there is no 'bug owner' :P [03:05] I still think we should do what I proposed. [03:05] It's one little workaround to remove grants on unsubscriptions [03:05] And preserves the current UI [03:05] wgrant: its up to you guys as a group, you're doing the work. [03:05] bug task owner(s) i think i meant [03:06] myself, I always try to go as directly as possible. [03:06] I don't see preserving the current UI as a goal, at any stage. [03:06] Sure [03:06] in this case i'm +1 with lifeless [03:07] Evolving the privacy rules gradually is a recipe for disaster. [03:07] especially if it's behind a flag [03:07] wallyworld_: It can't be behind a feature flag. [03:07] Not completely. [03:07] the ui i mean [03:07] It has to be shown to everyone the moment anyone can edit grants directly. [03:08] and then when we are happy with it, we can remove the triggers to revoke on unsubscribe [03:08] Or access becomes opaque [03:08] so what wgrant means, I think, is that when 'add without subscription' becomes available to *anyone*, then that access can't be hidden by the ff [03:08] so the change to the subscriber portlet, while it can be ff'd to start with [03:08] has to go live as soon as any manual grant - pillar or artifact scoped - is addable to the system [03:09] yes [03:09] So we cannot do gradual rollout. [03:09] so long as the portlet looks ok, what's the issue? [03:09] but again, this is a tiny, shallow UI change, consistent with current layout, and if we're committing to reviewing it to fix issues huwshimi/dan/mrevell identify, I don't see the issue. [03:09] you don't need it globally visible *until* you FF enable the 'add manual grant' feature [03:10] and that feature, is one that you don't need to enable until very late in the piece. [03:10] folk may want it sooner, but its up to you when its delivered. [03:10] So you can have as much design/UI review as you like before flipping the switch. [03:11] You can gradual-rollout the new subscriber portlet info in advance of gradual-rollout the ability to add grants [03:11] totally doable. [03:11] And totally another two weeks. [03:12] if we roll out the new ui, it will always be empty anyway atm due to the triggers [03:12] Indeed, the new section of the porlet will not be seen until manual grants happen. [03:12] So a gradual rollout there is not useful. [03:13] less useful [03:13] certainly [03:13] totally unuseful? don't think so. Missing table perms etc would be caught [03:14] poor query performance likewise. [03:14] wgrant: why another two weeks? [03:14] lifeless: Because that's how Launchpad works. [03:14] and i could hack in data for a screenshot [03:14] I can't usefully comment on such hyperbole [03:14] It's not hyperbole. [03:15] Attempting to iterate on interlocked UI and model changes *does not work*. [03:15] In Launchpad. [03:15] what model changes are expected here? [03:15] other than the trigger discard [03:15] wgrant: i call bullshit on that one. i did it fine for the +sharing page [03:16] lifeless: Moving access management into the application and out of the database, for one. [03:16] lifeless: we will only likely one one additional service method on the sharing service [03:16] for me to do the ui [03:16] wgrant: how is that different to the trigger being discarded? [03:16] wallyworld_: Permissions are going to be very awkward, but we'll see. [03:16] permissions for editing maybe [03:17] but that's a separate issue [03:17] the ui can still be done incrementally, first read only, the editable [03:17] and feedback can still be gotten [03:17] I am very wary about gradually evolving the privacy UI like that. [03:18] It's going to confuse people, and everyone already makes enough mistakes. [03:18] confuse who? [03:18] it will only be visible to select people [03:18] initially [03:18] us and product team perhaps [03:18] Sure, but we have to eventually roll this transitional edit UI out to everyone. [03:18] Only to change it again later. [03:19] or we could deploy it in one go like we said above [03:19] we change UI all the time, after we have data about how well it works. [03:19] wallyworld_: This UI we've discussed here won't be the final one. [03:19] wgrant: why not ? [03:19] well, let me rephase [03:19] beyond design requested changes [03:20] which isn't a guaranteed thing, because you might be doing it well enough first pass. [03:20] ... because there will be design requested changes. [03:20] so? we have to start somewhgere [03:20] and so long as it's functional and not too ugly [03:20] then it will be fine surely [03:20] retitle 'subscribers portlet' to 'sharing and notifications' - |your status | other people with different sorts of status| status2 | status3\ [03:20] (rotated 90' [03:21] ) [03:21] wallyworld_: I can see the blog post now [03:21] 'LP developers delivered on time' ? [03:21] lol [03:21] "Now, there are some people that have access but aren't subscribed. You can't add them, and they'll only appear sometimes, when something has happened. In a week this will change and everything will be like this" [03:22] that's not very helpful. i don't think it will happen like that at all [03:22] So, keep your current job :) [03:22] I don't think marketing is your forte. [03:22] hmm, that sounded meaner than I meant it. Sorry. [03:22] Heh [03:23] But seriously, I would say something like this: [03:24] 'The disclosure project has now enabled direct access control for the beta users of Launchpad. Users of that team can now grant access to private and security bugs without needing to create subscriptions. Less mail!. [03:24] So now we need the grant access UI too! [03:24] Awesome. [03:25] hmm. if only there were someone around who could write code [03:25] Code isn't the problem. [03:25] Code is easy. [03:25] *All users* of Launchpad will be able to see who has been granted access to a private or security bug. Once we've shaken down the system with our beta team, we will permit all users to use this facility." [03:25] UI when all our UI people have been stolen is the problem. [03:25] wgrant: hangon, take a step back please. [03:25] and a good long breath. [03:26] We won't be allowed to do this without mockups and user testing, and that will take at least 6 weeks. [03:26] My understanding is that noone will know this change is around until the grant access UI is enabled. [03:26] True or False. [03:26] The sharing management UI in general, yes/ [03:26] . [03:26] So True. [03:26] And, thats feature flagged. [03:26] Right? [03:26] Yes. [03:27] So, there is no need for the blog until that flag is toggled. [03:27] And the sketch I gave for a blog is sufficient when the flag is toggled - if the 'grant access' UI is the sharing management UI, thats sufficient. [03:27] Sure. [03:28] We this still means we can't finish the model until the new UI is designed. [03:28] so why did you wax sarcastic? [03:28] wgrant: so do the design for the UI. [03:28] Heh [03:28] Have you ever designed UI using the new process? [03:28] There is a difference between design and signoff. [03:28] we don't need the entire process to get started [03:28] or to have enough to do the model [03:28] We do. [03:29] I feel like you have a genuine point somewhere, but its getting hidden behind absolute statements and general hysteria. [03:29] This is exhausting trying to work through. [03:29] We cannot evolve the model until the UI enforces its display limitations. [03:29] sigh. i'm going to write code. see y'all [03:29] The point of my plan is to allow us to evolve the model before we have UI [03:29] In lifeless' plan, we cannot evolve the model until the new UI is turned on. [03:29] Which requires signoff. [03:29] turned on for everyone, that is [03:30] Your plan requires that where the model is implemented move around, but htat the model (in the general sense of 'rules of the system') stay the same for the same period of time. [03:31] Crucially, it allows us to completely stabilise the model code and work on more project privacy. [03:31] I was hoping to unblock you on a technically hairy transition, but if you believe you can't do it differently, thats your call. I'm not here to tell you how to do it. [03:31] Meanwhile the UI can be bikeshedded without blocking the rest of the world. [03:33] What I would do is nab dan or huw on skype tomorrow, talk them through the key things (show the non-subscribed grants, show a (-) if you can remove them), get them to ack that as a provisional thing, and move on. [03:33] LP UI will be bikeshedded for the next 50 years [03:33] (thats hyperbole :P) [03:37] Yea. We'll be using github by then. [03:37] * cody-somerville ducks. [04:28] wgrant: Does any of the bugsearch stuff still reference Bug.private ? [04:32] StevenK: Not the main stuff, but there are some stragglers. [04:33] Most were eliminated in a branch that landed a few hours ago. [04:33] wgrant: So we can't quite rip out Bug._private and friends yet [04:33] Sadly not. [04:34] Do you think it's worth putting together a silly branch that drops them just to see how much fallout is involved, or not yet? [04:34] Not yet. [04:35] * StevenK will continue to bash his head against IBranch. [04:35] Not until the legacy half of get_bug_privacy_filter is gone and we've grepped for [Bb]ug.private [04:35] jtv: O HAI [04:35] wgrant: Bug.private isn't dying, ._private is. [04:36] I mean DB [Bb]ug.private [04:37] Hm, I think Bug._private might actually be dead. [04:41] I believe it was meant to be unused except for compatibility with DB queries. [04:42] Right. I just can't see any others in the tree. [04:58] * StevenK reads the triggers related to Branch.transitively_private and prepares for a 12 hour drive. [05:13] wgrant: Hm, you were involved with jtv's testing on staging, does that qualify as QA for the purposes of r15218 and r15220 ? [05:14] StevenK: No, there have been some slight changes since. [05:15] He might be able to judge it sufficient, but I don't particularly want to. [05:15] wgrant: Yah, okay [05:43] aloha [05:56] czajkowski: I thought you were @ UDS ? [05:58] lifeless: yup I am indeed [05:58] lifeless: and I work on lp stuff in and around sessions and stuff so working now for a bit [05:58] but need jtv :/ [06:00] czajkowski: isn't it 11pm ? [06:00] lifeless: yup [06:08] StevenK: no, it's not quite the same thing. I've been trying to Q/A, but running into qastaging trouble [06:12] czajkowski: btw, feature work defects are never critical, unless they are oopsing [06:12] czajkowski: (because its planned work, its not an interrupt) [06:12] lifeless: ah ok thought they were as they're part of a new feature not working as the feature intended [06:13] sorry [06:13] czajkowski: no worries; also minor things like typo are at most high [06:13] (they don't stop people getting their work done) [06:14] czajkowski: 997346 should indeed be critical, because its a regression - but it should have the tag added [06:15] czajkowski: there is I think (and if not there should be), in the bug triage wiki docs a comment to the effect of 'all critical bugs have to have a tag explaining why' [06:15] e.g. timeout, or oops, or escalated, or regression [06:16] mmm, and looking closer [06:19] stub: hi thar [06:20] urgh [06:20] good morning :P [06:21] stub: ready for a catch up? [06:23] Ha. Can do, but not ready :) [06:24] skype or g+ ? [06:25] Hang on, need to steal my mike back. [06:26] I still haven't tried G+. [06:26] skype then :) [07:23] stub: hi. for some tests, is there an 'admin' user i can use like "with dbuser('admin')" which allows me to avoid 'you do not own this relation' errors. i have got it to work with 'ian' but obviously i can't commit that [07:24] wallyworld_: There is a 'testadmin' user [07:24] stub: it still complains when i try that [07:24] Hmm... 'you do not own this relation'. What are you trying to do? [07:24] stub: disable a trigger [07:25] oic. Need to be the real owner then. [07:25] as in 'ian' or whoever? [07:25] * stub checks to confirm if 'launchpad' or other [07:26] wallyworld_: os.environ['USER'] would be best for now [07:26] stub: cool. thanks [07:26] wallyworld_: Ideally, the 'testadmin' user should be a superuser if we are trying to do this sort of thing. Open a bug if you like. [07:27] stub: will do. i was hoping there'd be a symbolic admin user i could use [07:27] Currently it must just have full permissions on every object, which isn't quite a superuser. [07:27] ok [07:27] Yer. Didn't think of tests issuing DDL. [07:28] well, most don't :-) [07:28] i'll file a bug and add an XXX to my branch [07:29] No need for the XXX really - just a normal comment that os.environ['USER'] is a superuser (or our database setup scripts will completely fail) [07:30] stub: i was going to add the XXX so that when the bug i file is fixed, we can replace the os.environ with 'testadmin' [07:30] Sure. [07:31] wallyworld_, stub: Why not use postgres? [07:31] In test scenarious the current user should be able to auth as postgres, and postgres is both a superuser and the real owner. [07:31] wgrant: If we know that works for every dev, sure. [07:31] They have be able to auth as dozens of other users for the test suite to work. [07:32] So unless they've gone and named them all explicitly... [07:33] More that pg_hba.conf often starts with restrictive settings for connecting as postgresql (to ensure autovacuum etc. can work), followed by user settings. But our setup instructions say don't do that, so... [07:34] wallyworld_: Use 'postgres', no need for the bug or XXXX [07:34] ok, thanks [07:34] wallyworld_: We already switch to that user in other tests [07:35] wgrant: ta [07:35] i tried to find something but there's so many... :-) [07:39] wallyworld_: Can you review https://code.launchpad.net/~stevenk/launchpad/information_type-branch-privacy-trigger/+merge/105291 so I can land it? [07:39] stub: A review of https://code.launchpad.net/~stevenk/launchpad/db-branch-information_type-not-null/+merge/105282 would be lovely. [07:40] * stub looks [07:40] StevenK: ok, give me a minute [07:40] wallyworld_: It's tiny, only test changes. [07:41] StevenK: Are you using PG 9.1 yet? [07:41] StevenK: That col is indexed now, right? [07:41] stub: I'm not, but neither was the sampledata in db-devel [07:41] StevenK: Oh, ok. Thought that the 9.1 version had landed already. [07:42] cjwatson didn't regen sampledata [07:42] It looks to have in devel, so I will probably have fun with a merge [07:42] I did in devel a few hours back, with 8.4, because I knew there was more coming [07:42] diff didn't look that huge - was just checking [07:42] r=stub anyway [07:43] wgrant: stub did the index the day before the garbo job landed. [07:43] Should be deployable tomorrow. [07:43] Yeah, I thought I remembered that, but best to check. [07:43] wgrant: Except that I can't land it yet, since it requires the test fix that I wanted wallyworld_ to review. [07:43] * wallyworld_ is looking [07:44] StevenK: That's true. You may have to be lucky. [07:45] StevenK: r=me [07:45] Yes, rather. [07:46] wallyworld_: I considered changing it to the factory, but decided against it. [07:46] Why it does INSERT directly is beyond me, [07:47] good morning [07:49] StevenK: i can't recall why. i think it was because it was just testing the triggers and wanted to keep it simple [07:52] wallyworld_: Right, so there is a reason. [08:11] www.pretotyping.org - [08:24] Any reviewers in the house? I have 2 incremental MPs — https://code.launchpad.net/~jtv/launchpad/bug-994650-scrub-in-batches/+merge/105189 & https://code.launchpad.net/~jtv/launchpad/bug-994650-cache-potmsgsets/+merge/105193 === almaisan-away is now known as al-maisan === al-maisan is now known as almaisan-away [10:20] Does anyone have any tips on how to find, given a job class, what command line we use to run that job? [10:21] Ahhh, the trail leads to the culprit. Wow, we can't standardize this stuff soon enough. [10:29]