[00:34] wallyworld: bug 1342, whoa :) [00:34] <_mup_> Bug #1342: Can't delete spurious "Affects" lines (bugtasks) from bug reports

< https://launchpad.net/bugs/1342 > [00:34] poolie: are you happy or sad? [00:34] happy and impressed [00:35] happy yes, there's nothing to be impressed about [00:35] i would like if you qa http://launchpad.net/bugs/861979 some time so my oops fix can have a go at production [00:35] <_mup_> Bug #861979: utf-8 encode diffs attached to outgoing email < https://launchpad.net/bugs/861979 > [00:35] poolie: doing it now but it's being difficult [00:35] because not enough of that stuff works easily on qas? [00:35] branch scan job oopsed [00:35] poolie: We can't deploy until Monday at least. [01:02] wallyworld: hey [01:03] wallyworld: whats the implementation strategy you guys are planning for bug 1342 ? [01:03] lifeless: hello [01:03] <_mup_> Bug #1342: Can't delete spurious "Affects" lines (bugtasks) from bug reports

< https://launchpad.net/bugs/1342 > [01:03] lifeless: Magic [01:03] StevenK: sadly, I'm too jaded to consider that a great answer ;) [01:03] lifeless: log activity and delete unless it is the last task on a bug [01:04] do it via api initially [01:04] That isn't what lifeless meant. [01:04] And blocked via flag [01:04] yes [01:04] Personally, I'm picturing IBugTask.remove() [01:05] probably wants to be on Bug, but I'll let you guys figure that out [01:05] i haven't looked yet. doing qa and other administrivia [01:05] wallyworld: things to watch out for - conjoined masters will make 'last task on a bug' hard to determine [01:05] hmmm [01:06] Conjoinness rises its ugly head to bite us in the ass again [01:06] dumb question - what exactly is a conjoined master? [01:06] wallyworld: will notifications be sent to the context the task is removed from ? [01:06] lifeless: i expect so [01:06] wallyworld: your team can help you on the conjoined master thing :) - I mention it because its not obvious until you're in the trenches. [01:06] wallyworld: BugTask for Ubuntu, and then it's nominated for natty [01:07] * wallyworld needs to buy some gum boots [01:07] wallyworld: I think they should; as there isn't a LEP, I'm using this forum here to toss some constraints your way :) [01:07] StevenK: uhm, no. :) thats not a conjoined master [01:07] Oh [01:07] lifeless: I don't like it on IBug -- I prefer the idea of grabbing IBug.tasks, grabbing the one you want and calling .remove() or .delete() on it [01:08] StevenK: wallyworld: when you create a task for a *pillar directly* one task is made. If you also create a task for that pillars *development focus*, then those two tasks are shown as one in the Web UI, and are together a 'conjoined master' [01:08] StevenK: sometimes that will delete two tasks. [01:08] Why would it delete two tasks? [01:08] StevenK: so while you like the idea, I think it will be harder to code and harder to get right, if you put it on BugTask. Just saying. [01:08] wgrant: again, conjoined masters. [01:08] We don't have to continue treating them as conjoined for deletion. [01:08] wgrant: I will wait for a second while you headdesk. [01:09] wgrant: we have to show the difference in the UI then... [01:09] Delete one of the two, and it is magically not conjoined any more. [01:09] Magical! [01:09] wgrant: unless you delete the non-series task. [01:09] And? [01:09] There's no longer a conjoined slave. [01:09] No problem. [01:09] its not a valid state to have a series task with no non-series master [01:10] Isn't it? [01:10] I thought you argued a couple of months ago that it was. [01:10] When I was attempting to restrict it during my retargeting work. [01:10] I'm pretty sure we permit it now. [01:11] does the BugTaskSearch code have explicit code for dealing with conjoined masters? [01:11] Even if we want to forbid it, it's not directly related to conjoinment. [01:11] *Any* series task is affected. [01:11] Conjoinment is unspecial. [01:11] I argued that we have such states already, and that the conjoined thing messy, and that we shouldn't make it worse by breaking existing things [01:11] wgrant: thats true [01:11] wgrant: So let me phrase this differently: delete shouldn't let users create a state that they cannot already create. [01:12] Certainly. [01:12] Its my understanding that you cannot create a series task w/o a non-series same-pillar task, today. [01:12] (via web UI) [01:13] anyhow, long story short: beware the jabberwocky; send notifications to the folk the task was removed from; log enough that someone can undo brain damage; think about who can delete (the person who added it, the owner of the context, who else?) carefully. Have fun! [01:13] if the two tasks are shown as one in the web ui, if someone changes the assignee etc, which task gets updated? [01:14] wallyworld: both [01:14] lifeless: https://bugs.qastaging.launchpad.net/ubuntu/+bug/1234 [01:14] <_mup_> Bug #1234: Gina is an unmaintainable mess of command line options, environment variables and shell scripts < https://launchpad.net/bugs/1234 > [01:14] why do we need two task then? [01:15] wallyworld: the idea is that when a task nominated for the development focus is closed, its closed for the project as a whole too (in the absence of older series tasks) [01:15] wallyworld: the current implementation is just that, an implementation. [01:15] wallyworld: I would like to get rid of non-series tasks [01:15] lifeless: As for permissions, we agreed that bug supervisor + task creator is probably sensible. [01:16] wgrant: I'd suggest owner of the original task context, but that way lies madness [01:16] lifeless: Never let me hear you use the phrase "original task" again :) === StevenK changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: - | Critical bugs: 269 [01:22] sinzui: ping [01:27] feeling better? [01:37] me? mostly. [01:37] lifeless: You are a BAD MAN [01:37] BAD! [01:38] Been crook a few days. Yesterday it had mutated into a head cold but that seems to have cleared up now. [01:41] stub: ugh. Glad you're better :) [01:41] Just in time to be flooded tomorrow :-) [01:41] StevenK: getting things done in a tolerable fashion isn't bad. Cody can do that work unless it actually has holes in it. [01:42] lifeless: I don't think it's tolarable, I think it's a terrble hack [01:42] IPerson is already too fricken big [01:42] And advocating a celebrity? [01:42] StevenK: what are the consequences of it that make it intolerable ? [01:42] We want LESS celebrites, not more. [01:43] lifeless: That we are stuck maintaining it [01:43] until someone creates a launchpad-itself model object that such policy objects can link to, we need celebs [01:43] Cody or Tim drive it by and say "Excellent, the LP security model is out of our way, thanks" and we're stuck with the baby [01:43] There is also the issue of non-virt builders. [01:43] So, I object. [01:44] Ideally this would only grant them access to their own little world of fake-virt armel. [01:44] StevenK: you're rather they web scraped ? [01:44] lifeless: Web scraping is always better, because it's not an interface that we have to support. [01:44] wgrant: I'm not sure how to do that without deeper plumbing work; I was aiming to limit the scope of the permissions rather than rework [01:44] :) [01:45] lifeless: I'd rather we give them a restricted account with the privs they want, rather than other madness [01:45] But no one listens to me anyway, so meh [01:45] StevenK: this gives them a restricted account with the privs they want [01:45] StevenK: *I* listen to you - do you have a proposal on how that would work, and how does it compare in dev + maintenance effort to the other proposals ? [01:46] I wonder how close the secure panda cluster is. [01:46] stub: bug indices at 98% bloat :) [01:46] That would make this easier. [01:46] awesome [01:46] time to try pg_reorg? :) [01:46] oh, it's an index [01:46] I'll shutup now [01:47] elmo: :) [01:47] elmo: aieeeeeeeeeee [01:47] haha [01:47] https://code.launchpad.net/~stub/launchpad/replication/+merge/78914 has landed, but the scanner hasn't flagged it as merged. [01:47] hahaha [01:47] lifeless just googled pg_reorg [01:47] stub: db-devel vs devel [01:48] pg_reorg is for wimps. [01:48] I just did, that's *insane* [01:48] Oh, it landed on devel. [01:48] wgrant: nah... [01:48] Nevermind me then, something's just screwed. [01:48] Ah. [01:48] Will it have been logged somewhere? I'm not fussed but not sure if it should be raised somewhere. [01:48] https://code.launchpad.net/~stub/launchpad/replication [01:48] Ancient branch. [01:48] The scan probably timed out. [01:49] pg_reorg is that thing you use when you write to your database too much for vacuum to work right? [01:49] Because our branch scanner is fast and awesome and reliable and perfect. [01:49] (and want to die in a fire) [01:49] All my branches are ancient ;) [01:49] stub: Well, crucially, this one is missing 18 months of revisions. [01:49] Hah [01:50] Ahh [01:50] And the old scan is based on db-devel [01:50] The new one is probably devel. [01:50] So it has to rewrite tonnes. [01:50] I just crapped on https://code.launchpad.net/~allenap/launchpad/oneiric-librarian-bug-871596/+merge/79175 . Anyone disagree that this is a Storm bug and better to address there? [01:51] stub: Agreed. [01:51] That seems to pretty clearly be a DisconnectionError. [01:51] +1 [01:53] lifeless: Oh, I noticed this yesterday: [01:53] steven@liquified:~/launchpad/lp-branches/devel% head -n 1 LICENSE [01:53] Launchpad is Copyright 2004-2009 Canonical Ltd. [01:53] yes, copyright overheads are a PITA [01:53] lifeless: Should I JFDI the fix? [01:54] yeah [01:54] cron it for the 1/1/* [01:54] On my desktop? Not likely :-P [03:05] "But Robert tells us that the problematic team membership records are harder to clean up than regular old requests." [03:05] Clearly, both jtv and I are missing something. [03:07] ECONTEXT [03:09] wallyworld: \o/ at ajax batching [03:10] lifeless: seems to work well. if we get sorting solved, we can add it to the bug and blueprints tables on the milestones page [03:10] there's already a half done branch for that [03:11] whats the sorting issue? [03:11] when sortable.js is used for client side sorting, it doesn;t make sense if only a batch of results is rendered [03:11] well, thats what we do today right ? [03:12] the sorting doesn;t break, but it only sorts the current batch [03:12] how does avoiding page-loads make it worse? [03:12] yes but today for those tables there's no batching [03:12] oh, I see. [03:12] so the sort works on the entire results set [03:12] yes [03:12] so we need to catch the table header click and send a new query to the back end [03:12] can I make two suggestions [03:13] sure [03:13] rather than replacing the batch, consider - at least as an option - *appending* to it. [03:13] (and scroll down to the new bits) [03:14] lifeless: otp just a sec [03:14] I suspect, much of the time, that that would be closer to what users want [03:14] wallyworld: np [03:18] -> picking up lynne [03:19] lifeless: will ping you later [03:56] wgrant: Hai [03:56] *yawn* Morning [03:57] StevenK: Hello. [03:57] wgrant: I have a test, but I want to see that it creates an OOPS. [03:57] getLastOopsReport? [03:58] Or self.oopses perhaps. [03:58] Depending on how you do it. [04:02] Traceback (most recent call last):\nMixedVisibilityError\n [04:02] I am disappoint. [04:03] So I guess I need to inject a traceback into my raising() call [04:12] StevenK: no, if you do thats a bug somewhere else [04:12] StevenK: is it in a subprocess? [04:12] StevenK: if not, self.oopses is the thing to use. [04:13] lifeless: self.oopses looks good, but I want the full traceback [04:14] StevenK: it should be there [04:14] StevenK: if its not, something is mangling it [04:14] StevenK: report['tb_text'] should have it [04:15] 'tb_text': 'Traceback (most recent call last):\nMixedVisibilityError\n' [04:15] StevenK: as I said, something is mangling it [04:15] getUtility(IErrorReportingUtility).raising( [04:15] (MixedVisibilityError, MixedVisibilityError(), None), [04:15] request) [04:16] That's how I'm raising it, perhaps that is wrong [04:16] uhm, lets start over. What are you trying to do ? [04:16] lifeless: https://code.launchpad.net/~stevenk/launchpad/soft-oops-on-private-team-disclosure/+merge/79066 [04:16] there are no soft oopses [04:16] There are no *informational* OOPSes. [04:16] There can certainly be soft ones. [04:18] this has the potential to cause thousands of oopses [04:18] are you sure you want to do it ? [04:18] (for instance, an api collection with 75 members in it will be trimmed by lazr, and that one web API call will generate 75 oopses) [04:18] oh, and I see that you won't log from the API, which is good on one hand, but means you won't see it on the other [04:18] . [04:18] bloody ISP [04:18] lifeless: So, we want to turn this on for an hour or so and then see what happened [04:19] This is not a feature flag to leave enabled for too long [04:19] And it is purely for data gathering [04:19] So we can nail down the prime suspects, as it were [04:19] ok [04:19] so you actually need an *exception*: [04:19] try: [04:19] raise MixedVisibilityError('some text here') [04:19] I don't want to stop it [04:20] except MixedVisibilityError: [04:20] I just want to log it [04:20] ...raising() [04:20] StevenK: have faith that I understand your use case :) [04:21] try raise except results in 'tb_text': 'Traceback (most recent call last):\nMixedVisibilityError\n' [04:23] paste the updated code ? [04:23] lifeless: http://pastebin.ubuntu.com/707156/ [04:24] StevenK: nono, don't make the tuple by hand [04:24] sys.exc_info() [04:27] lifeless: Which tuple, sorry? [04:27] Oh, just toss sys.exc_info() rather than (MixedVi ... [04:27] http://paste.ubuntu.com/707159/ [04:28] Can someone who knows code imports help with this question? https://answers.launchpad.net/launchpad/+question/174071 [04:28] lifeless: Modulo the left bracket, I just did that myself, so great minds [04:29] StevenK: that should work better for you [04:29] 'tb_text': 'Traceback (most recent call last):\n Module lp.app.browser.tales, line 1327, in _report\n raise MixedVisibilityError()\nMixedVisibilityError\n' [04:30] Although I'm calling into the formatting API myself, so that should be okay. [04:31] lifeless: Thanks for the help. [04:31] StevenK: anytime [04:32] I was expecting more frames, but I shouldn't in this case. [04:41] * wgrant yearns for IArchiveCollection and IPublicationCollection === wgrant changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: - | Critical bugs: 267 [04:45] lifeless: batching - you want to append the next batch to the current batch? to create a table with twice as many rows (and a scroll bar)? doesn't sound quite right to me [04:46] Why not? [04:46] That's how most modern batchers work. [04:46] Start scrolling down, it automatically loads the next bit and extends the page. [04:48] that would be a significant rewrite of the current batcher though [04:48] well, the js etc [04:49] i was just wanting to do something backwards compatible and in line with the current implementation [04:49] at least for now [04:50] WIth launchpad's move to SOA, this might be an interesting read, if you folks haven't read it already - https://plus.google.com/112678702228711889851/posts/eVeouesvaVX [04:50] Yeah, that's been circulating around a bit. A rather impressive work. [04:51] Indeed [05:02] lifeless: Hai, Mr. OCR. [05:03] stub: has the branch transitively_private not null patch been applied to prod? [05:13] wgrant, wallyworld: Do either of you want to review my logging branch? [05:13] StevenK: i'll have a look [05:17] wallyworld: The NOT NULL patch is next in the DB queue. [05:17] wallyworld: Will probably be deployed on Monday, unless oneiric breaks stuff. [05:17] ok [05:17] just curious [05:18] The last few windows have been used for oneiric-critical cocoplum deployments, unfortunately. [05:18] StevenK: the method _report() - could it have a better name, like _report_visibility_leak() or something [05:19] wallyworld: I like the _ since it is internal, and I doubt it will remain there for long [05:19] StevenK: it's not the _, but the name [05:19] report [05:19] vs report_visibility_leak [05:19] Yes, alright [05:19] sorry [05:20] :%s/\(_report\)/\1_visibility_leak/ [05:20] Fixed [05:21] StevenK: i also think the test should ensure the ooops content is related to the mixed visbility error raised [05:21] at the moment, there's nothing to test that the oops is informational or is related to the error we are raising [05:21] it could be any oops [05:22] and we wouldn't know [05:22] self.assertTrue( [05:22] 'MixedVisibilityError' in self.oops[0]['tb_text']) [05:22] Rather than what is there, okay? [05:22] i think that works. is there a way to tell that it's a soft oops? [05:23] That the call returns u'', which I already test. [05:23] ah ok, since a hard oops would not return a result [05:23] it would just error out [05:23] Exactly. [05:24] cool. thanks for making the changes. r=me [05:30] wallyworld: its a desired feature, even though it is different to the non-js batch [05:30] wallyworld: non-js batch being the same as js-batch isn't a goal in and of itself :) [05:30] wallyworld: if you don't have time, thats fine. Consider though that the bugtask comment js batch works this way ;) [05:30] lifeless: sure. i'm just doing a bit at a time - small chunks of improvement [05:31] it was just a coding spike rather than a proper part of disclosure [05:31] ah cool [05:31] I think its great. [05:32] just tossing out nice to haves ;) [05:32] for what it is it works. backwrads compatible with non-js etc [05:32] i agree with the nice to haves [05:33] lifeless: while i have you - i wanted to put the deleteBugTask() on IBugTaskSet. it makes sense to me that the xyzSet interfaces be used to manage the lifecycle of the items they create etc [05:33] Any method on a Set that isn't set-based is a bug. [05:33] deleteBugTasks(), perhaps. [05:33] opinions are mixed about the *Set interfaces/instances [05:34] they are misnamed for much of what they do [05:34] wallyworld: Also, that doesn't work too well for the API. [05:34] As the Sets can't really be exported sensibly. [05:34] agreed. that was the issue i was going to aks about [05:34] This is most inconvenient. [05:34] putting it on there would be fine by me; I agree that it should take an interable of tasks, as future proofing. The API side of it is interesting though, as (IIRC) we haven't exposed the Sets on the PAI [05:34] we export IBranchSet [05:34] but it needs a url and default collectio method [05:35] We can't use a Set here. === almaisan-away is now known as al-maisan [05:35] As we need permission checking. [05:35] we pass in the user as part of the request [05:35] wgrant: that seems like a non-sequitur [05:35] deleteBugTask(bugtask, deleted_by) [05:35] lifeless: Until it's not awkward as hell, it's very relevant. [05:36] wgrant: its not awkward as hell ... [05:36] You have to manually work out the security stuff. [05:36] and we use call_with(who_deleted=REQUEST_USER) [05:36] Sure, we can get the user in there. [05:36] do you mean 'you'd have to check the security by probing rather than security adapters' ? [05:36] that seems to be the pattern used everywhere else [05:36] But then you have to manually determine and invoke security adapters. === al-maisan is now known as almaisan-away [05:38] i like the idea of a separate manager type interface to control the lifecycle of objects [05:39] it fits more with a service based design where domain objects encapsulate state and services manage the workflow etc [05:39] * StevenK attempts to stop his eyes glazing over [05:40] so we need to stick the deleteBugTask() method somewhere other than on IBugTask itself or even IBug [05:40] StevenK: you know you love this stuff [05:40] wallyworld: the current pattern in LP, for good or ill, is destroySelf() [05:40] Love to see it burn, sure. [05:40] yuk! [05:40] wallyworld: I would introduce BugTaskSet.delete(sequence_of_tasks) [05:40] IBugTask.destroySelf() [05:40] Then BugTask.destroySelf calls that. [05:40] nooooooo [05:41] domain objects do not call services [05:41] In our current architecture they do. [05:41] wallyworld: we don't have a domain/service split today [05:41] domain objects should just encapulate state and nothing more [05:41] Yes, it is bloody awful. [05:41] But this is Launchpad. [05:41] lifeless: yes, but do we want to propogate bad edesign? [05:41] We have no choice here. [05:41] wallyworld: introducing one incrementally is fine, but I would like to have some confidence we won't get lots of stubby attempts that don't flow together [05:42] sure [05:42] The current API and security design mandates that the method be on the object. [05:42] so given we have an addTask method on IBug, i could put a deleteTask method there too [05:42] and then it can update the heat etc [05:42] wgrant: Did you see the new design github deployed? It reminds me of old Launchpad :P [05:42] I don't think destroySelf is 'bad design' - its the logical consequence of the design of the zope stack. [05:42] just like addTask does [05:42] wallyworld: -the entire stack- [05:42] nigelb: Indeed, with the tabs along the top. [05:43] lifeless: destroySelf is ok, but that should not be exposed via the service layer [05:44] wallyworld: What service layer? [05:44] The API, I guess [05:44] wgrant: i'm generising [05:44] LP has no layers. [05:44] wallyworld: consider the interactions between ORM-objects-are-content-objects, security adapters and highly delegate-to-content-object design [05:44] wallyworld: if you grok that, and still want to add a service layer as part of this work, you have my blessing. [05:45] wallyworld: few folk do grok all those interactions though :( [because its bloody vicious] [05:45] lifeless: i'd like to move to a different design which separates out content from behaviour from workflow modelling from business process modeliing from orm etc [05:45] but that's in an idea lworld [05:46] i realise we have something different [05:46] largely dictated by the design of the zope stack i guess [05:46] lifeless: wgrant: so you happy if i add a deleteBugTask method to IBug? [05:47] and if so,i would raise an exception if a bug task were passed in that didn;t belong to the bug? [05:47] wallyworld: I don't think it should be on the bug. === jtv is now known as jtv-eat [05:47] addTask is there because it's adding a task to the bug. [05:47] but the bug needs to be involved because it needs to update bug heat [05:47] I think it should be on the task [05:47] Fucking bug heat. [05:48] Kill it. [05:48] so if it is on the bug task, i would need to update bugg heat still [05:48] and call back to the parent bug [05:48] sigh [05:48] Well, ideally the *Set method would do that. [05:48] In a batch fashion. [05:49] yes, but to expose the IBugSet method - it needs a url and a default collection method [05:49] and there isn;t one at the moment [05:49] Sure, which is why you put a trivial method on BugTask to delegate to it. [05:49] Matching the rest of LP's deletable objects. [05:49] ok, but i will have to have a shower afterwards. that is just sooooo wrong [05:49] i was thinking the other day about bug 678090 - i'd like to see how many people are actually affected by bugs, across dupes [05:50] <_mup_> Bug #678090: Affected people from duplicates aren't included in the master bug's affected count

< https://launchpad.net/bugs/678090 > [05:50] stub, or someone, could you comment on my db performance question on the bug [05:50] basically, is counting them at run time going to be fast enough, or should we maintain a count? [05:50] wgrant: StevenK: to do it on bugtask seems the best option, but it just gets the layering *completely* wrong :-( [05:50] wallyworld: It's as good as it can get now. [05:51] wallyworld: Until we end up with a sensible architecture. [05:51] "lol" [05:51] Which may happen in 10 years when we're out of criticals :) [05:51] yeah :-( [05:51] But don't worry, there's nothing wrong :) [05:52] wgrant: i have to run away and pick up the kid from school. i doubt we'll see any qastaging email today :-( [05:52] How do I find which things are exposed over the API and wwhich arent? [05:52] nigelb: search through http://launchpad.net/+apidoc ? [05:53] nigelb: they are annotated and also listed in the webservice.py modules [05:53]