| wgrant | StevenK: Well, it's completely valid. | 00:01 |
|---|---|---|
| wgrant | StevenK: You're just invalidating the assumptions it's based on. | 00:01 |
| wgrant | StevenK: Is that one of the tests that I said you should keep? | 00:02 |
| wgrant | StevenK: The key is that not all structsubs will produce notifications. | 00:02 |
| wgrant | The one tested in that test does, because it uses the owner for convenience. | 00:02 |
| wgrant | So change the test to not subscribe the owner, and it will hopefully pass. | 00:02 |
| wgrant | lp.bugs.model.tests.test_bug.TestBugPrivateAndSecurityRelatedUpdatesPrivateProject.test_transition_to_private_grants_subscribers_access | 00:03 |
| wgrant | What a test name. | 00:03 |
| StevenK | wgrant: So if I change the test to structually subscribe a different person, then it is pretty much lp.bugs.tests.test_bug_notification_recipients.TestBugNotificationRecipients.test_private_bug_with_structural_subscriber | 00:04 |
| wgrant | "This seems like a reasonable integration test to retain. You've added a test that sharing => notification in test_private_bug_with_structural_subscriber_with_access, but unless I've missed one this is the only one that tests !sharing => !notification." | 00:05 |
| wgrant | I missed one :) | 00:05 |
| StevenK | Right | 00:06 |
| StevenK | wgrant: That doesn't happen often. :-) | 00:06 |
| wgrant | Hah | 00:06 |
| wgrant | You should always doubt me. | 00:06 |
| wgrant | Unless I'm right. | 00:07 |
| StevenK | wgrant: I've addressed all of your comments, aside from the two related to (368-369, 392-396) | 00:08 |
| StevenK | Those two can wait until after breakfast. | 00:09 |
| wgrant | StevenK: What breaks when you filter direct subscribers? | 00:09 |
| wgrant | That bit is not negotiable. | 00:09 |
| wgrant | 392-396 might be | 00:09 |
| StevenK | wgrant: No fair engaging me with questions when my stomach is threatning to break out and find breakfast on its own. | 00:10 |
| wgrant | StevenK: that sounds beneficial. | 00:10 |
| wgrant | It means you can keep working on this branch while your stomach eats. | 00:10 |
| StevenK | wgrant: Hah | 00:35 |
| StevenK | wgrant: So I think your comment of "The function didn't previously return nothing for private bugs, ..." is wrong, since I think it was never actually called for private bugs. | 00:35 |
| wgrant | StevenK: It may not have been. | 00:52 |
| wgrant | StevenK: But you'll need to check other callsites. | 00:52 |
| StevenK | I'm about to filter direct subscriptions so I can show you the breakage. | 00:53 |
| wgrant | Great. | 00:53 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1120572/ is the crux of the breakage | 00:57 |
| wgrant | StevenK: Why does that happen? | 00:58 |
| StevenK | wgrant: Like I said on the call, I didn't investigate. | 00:59 |
| StevenK | I figured out it works if I don't filter direct subscriptions and left it at that. | 00:59 |
| wgrant | Right, you should figure that out. It's probably because it adds grants for direct_subscribers | 01:00 |
| wgrant | Depending on what else calls direct_subscribers, you may not want to filter direct_subscribers itself | 01:01 |
| wgrant | But a wrapper | 01:01 |
| StevenK | wgrant: direct_subscribers is a wrapper around direct_subscriptions | 01:01 |
| wgrant | Sure | 01:02 |
| wgrant | But it's a wrapper for a different purpose | 01:02 |
| StevenK | Hm, nothing seems to call it directly | 01:02 |
| wgrant | # Grant the subscriber access if they can't see the bug. | 01:03 |
| wgrant | subscribers = self.getDirectSubscribers() | 01:03 |
| wgrant | That might be relevant | 01:03 |
| StevenK | Yes, I was about to dig into callsites of getDirectSubscribers() | 01:03 |
| StevenK | Oh, is that working around a bug in the sharing service? | 01:05 |
| wgrant | Probably not. Why? | 01:05 |
| StevenK | I was guessing self.getDirectSubscribers() == [] | 01:06 |
| StevenK | Looks like I'm wrong | 01:06 |
| wgrant | The sharing service doesn't know about subscriptions. That's not its job. | 01:07 |
| wgrant | Now | 01:07 |
| wgrant | Filtering Bug.direct_subscribers directly is probably wrong | 01:07 |
| wgrant | Since people are still subscribed | 01:07 |
| wgrant | They just can't see it. | 01:07 |
| StevenK | wgrant: Oh? Shall I substract it from also_notified_subscribers then? | 01:09 |
| wgrant | StevenK: They need to be notified if they have access | 01:11 |
| wgrant | Like also_notified_subscribers | 01:11 |
| wgrant | also_notified_subscribers is only used for notification purposes | 01:11 |
| wgrant | It is only meaningful for notification purposes. | 01:12 |
| wgrant | So it is sensible to filter that there. | 01:12 |
| wgrant | But direct_subscribers isn't just used for notification. | 01:12 |
| StevenK | Right, so you *do* want me to filter it in also_notified_subscribers | 01:12 |
| wgrant | Not subtract it from :) | 01:12 |
| StevenK | Just trying to work out how to filter ... | 01:13 |
| wgrant | StevenK: In fact | 01:13 |
| wgrant | Hm | 01:13 |
| StevenK | self.direct_subscribers_at_all_levels is not the place, since that backs directly onto direct_subscriptions | 01:13 |
| wgrant | so | 01:14 |
| wgrant | direct_subscribers is just used for exclusion, it seems | 01:14 |
| wgrant | I misread. | 01:14 |
| wgrant | The real thing you need to filter is the getDirectSubscribers call in getBugNotificationRecipients | 01:14 |
| StevenK | Hmmm | 01:16 |
| StevenK | Which is in the guts of IBug itself, the rest of my playing has been in BugSubscriptionInfo | 01:16 |
| StevenK | wgrant: So not getDirectSubscribers() itself? Since we already figured out that fiddling with recipients after the fact is Bad. | 01:17 |
| wgrant | In related news, methods that mutate their arguments are bad. | 01:22 |
| StevenK | Who woulda thunk it | 01:23 |
| wgrant | StevenK: transitionToInformationType and a couple of other places use getDirectSubscribers for non-notification purposes. | 01:24 |
| StevenK | Right, but trying to remove things from BugNotificationRecipients afterwards is what led to a rollback and this branch in the first place. | 01:26 |
| wgrant | Certainly. | 01:28 |
| wgrant | I didn't suggest filtering afterwards :) | 01:28 |
| wgrant | I just implied that you can't do it directly and unconditionally in getDirectSubscribers | 01:28 |
| StevenK | wgrant: Modulo some clean up, http://pastebin.ubuntu.com/1120614/ | 01:29 |
| wgrant | StevenK: Right, that's probably reasonably sensible. | 01:31 |
| StevenK | Now to make it work | 01:33 |
| * StevenK tries to remember how set operations work | 01:34 | |
| StevenK | wgrant: I can't just set direct_subscribers, since it expects to be a BugSubscriptionSet, I'm trying to remember the set operation for "Remove everything in set A that isn't in set B" | 01:36 |
| wallyworld_ | sinzui: i am thinking that it would be better to add a remove icon next to the bug dupe link in the portlet (like is done for removing subscribers on the same page) rather than add a new link to the picker. that way the portlets on the page behave consistently | 01:37 |
| wgrant | wallyworld_: It's a bit different, though | 01:37 |
| StevenK | Which is intersection(), I thinjk | 01:37 |
| wgrant | wallyworld_: Since with the dupe thing you can only have one, and you can change it. | 01:37 |
| wgrant | wallyworld_: It's more similar to assignee that subscriber. | 01:37 |
| wgrant | s/that/than/ | 01:38 |
| === nigelb is now known as nigel-cloud | ||
| wallyworld_ | which i think is silly also to have to open a picker to remove | 01:38 |
| wallyworld_ | there should be a remove icon next to the pencil icon for assignees etc | 01:38 |
| wgrant | StevenK: >>> s = {1, 2, 3} | 01:39 |
| wgrant | >>> s.difference_update({1, 3}) | 01:39 |
| wgrant | >>> s | 01:39 |
| wgrant | set([2]) | 01:39 |
| wgrant | wallyworld_: Maybe, yeah | 01:39 |
| wgrant | wallyworld_: But we probably want to be consistent. | 01:39 |
| StevenK | wgrant: intersection_update(), since I want the intersection of the two sets. | 01:39 |
| wallyworld_ | yeah, i can't decide | 01:39 |
| wgrant | StevenK: Oh, bah, missed "isn't" | 01:40 |
| StevenK | But BugSubscriptionSet doesn't implement that. | 01:40 |
| wgrant | You might have to get to it before it objectifies it. | 01:40 |
| wgrant | Or alter getDirectSubscribers' other callsites to not call getDirectSubscribers | 01:40 |
| StevenK | No, I need to recreate it, BugSubscriptionSet is a frozenset | 01:41 |
| StevenK | ... somehow | 01:41 |
| wgrant | Due to @freeze(BugSubscriptionSet), porbably. | 01:41 |
| StevenK | The following table lists operations available for set that do not apply to immutable instances of frozenset: | 01:42 |
| StevenK | Which intersection_update() is in | 01:42 |
| wgrant | Sure | 01:43 |
| wgrant | A frozenset is frozen | 01:43 |
| wgrant | So updating it doesn't make an awfully large amount of sense. | 01:43 |
| StevenK | Yes | 01:43 |
| StevenK | direct_subscribers = BugSubscriberSet( | 01:44 |
| StevenK | direct_subscribers.intersection(filtered_subscribers)) | 01:44 |
| wgrant | wallyworld_: In r15569 you changed bug filing to not respect extra_data.private if the user is a bug supervisor. Do you recall why? | 01:48 |
| wgrant | (it probably means that apport-filed bugs for ~ubuntu-bugcontrol end up public unless someone's watching carefully) | 01:49 |
| wallyworld_ | hmmm. not of the top of my head. i'll see if i can remember | 01:49 |
| StevenK | HAHAHA | 01:50 |
| * StevenK notices a typo in accesspolicy | 01:50 | |
| wgrant | StevenK: Where? | 01:50 |
| StevenK | def revokeByArtifact(cls, artifacts, grantees=None): | 01:51 |
| StevenK | - """See `IAccessPolicyGrantSource`.""" | 01:51 |
| StevenK | + """See `IAccessArtifactGrantSource`.""" | 01:51 |
| StevenK | cls.findByArtifact(artifacts, grantees).remove() | 01:51 |
| wgrant | Copy-paste errors in docstrings of boilerplate methods. How novel :) | 01:52 |
| wallyworld_ | wgrant: i can | 01:57 |
| wallyworld_ | can't recall whyt thaT CHANGE WAS MADE | 01:57 |
| wallyworld_ | arg, caplock fail | 01:57 |
| wgrant | wallyworld_: Yeah, I can't see a good reason. I'm replacing that code, so I'll ignore it :) | 01:58 |
| wgrant | Thanks. | 01:58 |
| wallyworld_ | ok, thanks | 01:59 |
| wallyworld_ | too bad we didn't have tests that failed | 01:59 |
| StevenK | wgrant: Diff updated, can you cast your eye over it again? | 02:02 |
| === nigelbabu is now known as nigelb | ||
| wgrant | StevenK: Sec | 02:34 |
| wgrant | wallyworld_: Why's +filebug's information type widget part of bugtarget-filebug-guidelines.pt, rather than with the rest of the form? | 02:34 |
| wgrant | I don't really understand why that view is separate at all. I wonder if it might be because of the way ProjectGroup:+filebug used to work | 02:35 |
| wgrant | Which means it can probably be merged. | 02:35 |
| wallyworld_ | from memory that's how it was originally all set up | 02:35 |
| wallyworld_ | that's where the old privacy/security checkboxes were | 02:35 |
| wgrant | Huh | 02:35 |
| wgrant | Indeed. | 02:36 |
| wallyworld_ | it's all a a bit of a mess | 02:36 |
| wgrant | Ah, of course | 02:36 |
| wgrant | Because the security_related checkbox had the name of the security contact in it | 02:36 |
| wgrant | So it was project-dependent. | 02:36 |
| wallyworld_ | yes, makes sense | 02:36 |
| wgrant | But ProjectGroup:+index just redirects now, so that can all be merged. | 02:36 |
| * wgrant merges. | 02:36 | |
| === beuno_ is now known as beuno | ||
| StevenK | wgrant: How about now? | 03:16 |
| StevenK | Since your 'sec' has turned out to be a description of how long it will take you to get distracted, not how long it will take you to look at my MP. | 03:16 |
| nigelb | lol | 03:17 |
| wgrant | StevenK: r=me | 03:45 |
| cr3 | is there a way to have the status of a bug in launchpad set to fix released automatically when a package is automatically built from recipe? | 03:47 |
| wgrant | cr3: No. You'd have to write a launchpadlib script to do that. | 03:49 |
| wgrant | Recipes are normally used for daily builds, which aren't usually "releases" | 03:49 |
| cr3 | wgrant: that makes sense actually, thanks! | 03:50 |
| StevenK | wgrant: I guess that branch doesn't fix bug 901548, but lays the groundwork? | 03:53 |
| _mup_ | Bug #901548: launchpad does not send emails to the assignee of private bugs <bugs> <disclosure> <email> <privacy> <sharing> <Launchpad itself:In Progress by stevenk> < https://launchpad.net/bugs/901548 > | 03:53 |
| wgrant | StevenK: I think it probably fixes it implicitly. | 03:53 |
| wgrant | I'm pretty sure I tested that case, in fact. | 03:54 |
| wgrant | But that was like 4 days ago | 03:54 |
| StevenK | Then I'll link the bug and branch, thanks. | 03:54 |
| wgrant | Like | 03:54 |
| wgrant | Why wouldn't it fix it? | 03:54 |
| wgrant | It seems like it should | 03:55 |
| wgrant | Since the assignee is in also_notified_subscribers | 03:55 |
| StevenK | That's a point. | 03:55 |
| StevenK | stub: Hai! https://code.launchpad.net/~stevenk/launchpad/db-bugsubscriptionfilter-itype/+merge/117003 would love an review. | 03:58 |
| stub | StevenK: What are the different information types again? | 04:01 |
| lifeless | huey duey and louie | 04:01 |
| lifeless | stub: has mthaddon been in touch w.r.t. django session cleaning mark 2 ? | 04:01 |
| StevenK | stub: PUBLIC, PUBLICSECURITY, PRIVATESECURITY, USERDATA and PROPRIETARY | 04:01 |
| stub | lifeless: Not yet | 04:02 |
| StevenK | stub: The idea being that someone can structurally subscribe to all PRIVATESECURITY bugs. | 04:02 |
| lifeless | stub: ok, so - we're triggering mass locks in vacuum, theory is its scanning for pages to fill into but the table is so big... it takes ages *and* holds locks while it does the scan. | 04:03 |
| lifeless | stub: so the cleanups are on hold; I've suggested we do a bait and switch to a new table | 04:03 |
| lifeless | with a background clone of live sessions + a indexed copy of new sessions only during a FDT-like window. | 04:04 |
| wgrant | lifeless: That looks like oops-tools success. | 04:05 |
| stub | StevenK: Yeah, I was thinking humans would be interested in 'all', and 'security'. | 04:05 |
| StevenK | stub: It is following the currently established pattern, see BugSubscriptionFilterStatus and BugSubscriptionFilterImportance | 04:05 |
| stub | StevenK: Yeah, model is fine. I'll leave it up with you guys to design the UI. | 04:05 |
| * StevenK digs for a bug | 04:05 | |
| stub | StevenK: My initial reaction is we have a confusing UI if it allows someone to subscribe to just publicsecurity bugs, or just userdata bugs. | 04:05 |
| stub | StevenK: But I'm not reviewing that :) | 04:05 |
| stub | lifeless: How did you confirm vacuum is taking locks? | 04:06 |
| lifeless | stub: lock dependency graph shows it holding locks, strace shows it reading lots of data. | 04:07 |
| stub | lifeless: what sort of locks? | 04:07 |
| lifeless | I don't recall, I think they were row level (but rows that sessions were trying to use) | 04:08 |
| stub | Right, autovacuum is supposed to not affect anything and release locks and back off if other stuff needs them. | 04:09 |
| lifeless | it backs off at the deadlock detection interval | 04:09 |
| lifeless | it definitely takes locks out :) | 04:09 |
| stub | These are 8.4 though... | 04:09 |
| StevenK | wgrant: Hmmm, is there a bug for filter by InformationType? | 04:09 |
| StevenK | wgrant: I thought there was one, but I'm having trouble finding it | 04:10 |
| StevenK | Ah, found it | 04:10 |
| StevenK | stub: Can haz stamp on the MP? | 04:12 |
| stub | lifeless: Some vague hints that it might be our use of the CURSOR... | 04:13 |
| StevenK | lifeless: Why repeated oops tools mails? | 04:14 |
| stub | StevenK: Do you really want that 'id' column? | 04:15 |
| StevenK | stub: It's a many-to-many relationship, I think it's needed | 04:15 |
| wgrant | StevenK: Testing the new report-specific addresses | 04:15 |
| wgrant | StevenK: So we don't get U1 | 04:15 |
| wgrant | StevenK, stub: It's not necessary, but it's probably best to match the others | 04:16 |
| wgrant | (which have the ID, for reasons that are not clear) | 04:16 |
| StevenK | wgrant: Ah, testing on production. WCPGW. | 04:16 |
| stub | StevenK: Oh. Why isn't the (filter, information_type) unique? | 04:16 |
| StevenK | stub: Because I was making it match Status and Importance, but that's not the best answer. | 04:19 |
| stub | lifeless: yes, the cursor we use to maintain the list of rows to remove can pin blocks. " If, for example, someone leaves a cursor open, the data block to which the cursor refers is busy (the technical term is "pinned") and can't be vacuumed until the cursor is closed or moved to a different block" | 04:19 |
| stub | StevenK: At the moment, you can have two identical rows. That means your queries are going to need DISTINCTS all over the place to remove the redundant rows when you join with that table. | 04:20 |
| stub | StevenK: There is no reason for (filter, information_type) to not be unique, since there is no other actual information in the table (id doesn't count as information) | 04:22 |
| StevenK | stub: So, maybe we fix Status and Importance in the same patch | 04:22 |
| stub | And if it is unique, we can drop the id column too making (filter, information_type) the primary key. | 04:22 |
| * StevenK tries to cook up a query showing if there is anything in BugSubscriptionFilterStatus that is duplicated. | 04:23 | |
| * StevenK reaches for mawson | 04:25 | |
| lifeless | stub: yes | 04:26 |
| lifeless | stub: so you think its the cursor, and vacuum is a bystander ? | 04:26 |
| lifeless | stub: this conflicts with some evidence we have - killing the cleanup process doesn't unstick servers that this happens to. | 04:26 |
| lifeless | stub: I don't know why. | 04:27 |
| lifeless | bah | 04:27 |
| lifeless | StevenK: I don't know why (oops-tools 2nd mail) | 04:27 |
| stub | lifeless: Have you killed the backend server processes when you have killed the cleanup process? | 04:27 |
| stub | They tend to hang around with postgresql unfortunately. | 04:27 |
| lifeless | I think so. Not 100% | 04:28 |
| lifeless | stub: what do you think of the proposed approach anyhow ? | 04:28 |
| stub | lifeless: oh, proposed approach is fine. Might take some downtime though unless we much around with triggers and stuff. | 04:28 |
| stub | lifeless: Saves us having to run a CLUSTER after to remove the miles and miles of empty pages. | 04:29 |
| StevenK | stub: So it looks like there are guards in place in the model code that avoids the need for UNIQUE. | 04:30 |
| StevenK | stub: (See -ops) | 04:30 |
| lifeless | stub: it looks to me like we could get 4-5 seconds downtime | 04:30 |
| lifeless | stub: just need an index on the session expiry | 04:31 |
| StevenK | wgrant: Did you see https://www.djangoproject.com/weblog/2012/jul/30/security-releases-issued/ ? | 04:31 |
| stub | StevenK: Right. So why do we want the data model to allow redundant rows in the table that are totally indistinguishable? | 04:31 |
| StevenK | stub: We don't, and it doesn't at the moment, is what I'm saying. | 04:31 |
| lifeless | stub: step A) index that concurrently; B) copy live sessions, C) copy live sessions again but only those with an expiry time >> (time of previous query + expiry window); D) downtime starts | 04:31 |
| stub | lifeless: Which should be added anyway for regular cleaning runs | 04:31 |
| stub | StevenK: The proposed data model does. | 04:32 |
| StevenK | stub: You're having a deep discussion with lifeless, prod me when you're done. | 04:32 |
| lifeless | stub: E) copy live sessions again as in C; F) switch tables; G) downtime finishes. | 04:32 |
| stub | lifeless: Just going downtime & copying the relevant records across is possibly best. Can do a timing. | 04:35 |
| stub | But it isn't the underlying issue, which is that people are seeing behavior in autovacuum that other people don't see. | 04:36 |
| lifeless | stub: other folk do see it, rarely. | 04:38 |
| lifeless | stub: and you may have found a cause :) | 04:38 |
| stub | Yes, so fixing the django script to not hold open a cursor might be the best approach. | 04:39 |
| stub | That is just a guess though, based on vague comments. Reports of problems are rare, vague and usually for different situations (like the one I'm reading now, to do with TRUNCATE) | 04:42 |
| lifeless | stub: so, see the incident report on the wiki for more info | 04:44 |
| lifeless | stub: I'd like us to design around the risk, if we can, as extended downtime sucks :) | 04:44 |
| stub | lifeless: What risk are we designing around? | 04:45 |
| lifeless | stub: that we don't understand the root cause of the problem | 04:45 |
| lifeless | stub: e.g. that it really is just vacuum itself, such as the hidden lock documented to be in 8.4 | 04:45 |
| stub | You can't design around vacuum. Vacuum is an integral and required part of PostgreSQL. | 04:46 |
| stub | You can't even turn it off entirely, despite the magic option in postgresql.conf that should only ever be used in extremely rare use cases. | 04:46 |
| stub | Vacuum getting involved here would be a symptom, not a problem. | 04:47 |
| stub | I suspect that the problem is the cursor I old open for up to an hour, which is like a long transaction in many ways. | 04:47 |
| lifeless | stub: we can design around vacuum - e,.g. the bait and switch approach :) That said, its going to be up to you:) | 04:48 |
| stub | It is just intuition, but we should probably fix it anyway since we will hopefully not need hour long transactions with pg_dump anyway. | 04:48 |
| stub | lifeless: Guess what happens when you fill the new table with data? It gets vacuumed... | 04:49 |
| stub | well... analyzed. | 04:49 |
| stub | which is autovacuum | 04:49 |
| lifeless | stub: yes, but we've no reason to think autovaccuum is intrinsically a problem | 04:50 |
| lifeless | stub: the data we have is a) big table with b) lots of deletes and c) lots of writes | 04:50 |
| lifeless | I agree we should fix the cursor *too*, because we need to keep the tables trim. | 04:50 |
| stub | We also need to run this script daily or more often or we end up in the same situation. | 04:51 |
| lifeless | There is also the question of why we're getting millions of sessions | 04:51 |
| lifeless | I guess I'm saying its not either-or. | 04:51 |
| stub | Yes, I agree. | 04:51 |
| stub | It would be nice if we can avoid the risks of swapping a new table into place, and having to write the script to do it. | 04:53 |
| StevenK | stub: So if I'm going to change BugSubscriptionInformationType, I'd rather also change BugSubscriptionFilter{Status,Importance} to match. And I don't think I'm up for two primary key swaps. | 04:56 |
| wgrant | The tables probably have about $notverymany rows each, so it's easy. | 04:57 |
| StevenK | wgrant: Haha. 108 in status and 46 in importance | 04:59 |
| wgrant | That's even fewer than I expected. | 04:59 |
| wgrant | I thought maybe a few hundred each | 04:59 |
| StevenK | That's on DF, so condiment to taste | 05:00 |
| StevenK | wgrant: How do I deal with it in the model, though? | 05:00 |
| wgrant | StevenK: __storm_primary__ | 05:00 |
| wgrant | It'll need to be in a separate branch | 05:01 |
| wgrant | Which can land in about 5 minutes | 05:01 |
| StevenK | wgrant: Right, but I do land that first and then the db patch that will DROP COLUMN? | 05:01 |
| wgrant | Right | 05:02 |
| wgrant | Land+deploy the app PK change first | 05:02 |
| wgrant | Or the app will murder you | 05:02 |
| * StevenK sorts that out, since it's easy | 05:02 | |
| stub | StevenK: We can go with the id column, but unless there is a reason it is better to drop it as it is unnecessary and slower. | 05:16 |
| stub | I don't recall if there was a rationalization for it with BugSubscriptionInformationType | 05:16 |
| wgrant | At least we're not in too much danger of wraparound :) | 05:16 |
| stub | erm... bugsubscriptionfilerstatus, importance | 05:18 |
| StevenK | stub: So I've put up a model change for status and importance that will allow us to drop their IDs | 05:20 |
| StevenK | Then I'm not sure what needs to happen on the DB side | 05:20 |
| stub | ALTER TABLE foo DROP COLUMN id; DROP INDEX foo_filter_status_idx; ALTER TABLE foo ADD CONSTRAINT foo_pkey PRIMARY KEY (filter, status); | 05:22 |
| wgrant | Yeah | 05:22 |
| wgrant | Can't use USING, since the index is non-unique for no good reason. | 05:23 |
| StevenK | Right | 05:23 |
| StevenK | And adding the constraint gets us the index too, with unique for good measure. | 05:23 |
| wgrant | Right | 05:23 |
| wgrant | A primary key is basically just a UNIQUE index without any nullable columns. | 05:24 |
| StevenK | wgrant: https://code.launchpad.net/~stevenk/launchpad/no-more-id-for-bsfs/+merge/117374 | 05:24 |
| wgrant | (in postgres) | 05:24 |
| stub | StevenK: You're happy with this, or do you have deadlines? I'd like the model fixed, but it seems to have been broken for some time already. | 05:24 |
| StevenK | stub: See above :-) | 05:24 |
| wgrant | StevenK: You've checked around to see that nothing references them? I suspect it's safe, but there might be something. | 05:25 |
| wgrant | Also, you can't spell = | 05:25 |
| wgrant | In the first hunk | 05:25 |
| StevenK | Oh bloody hell, how did I miss THAT | 05:25 |
| StevenK | wgrant: A bzr grep -i 'BugSubscriptionFilterStatus.id' showed up nothing | 05:26 |
| wgrant | StevenK: Have you run the bugsubscriptionfilter tests? | 05:26 |
| wgrant | Although I guess an ec2 run won't hurt | 05:27 |
| wgrant | Since we can't deploy until tomorrow morning anyway | 05:27 |
| wgrant | StevenK: Also, it might want filter_id rather than filter. I forget. | 05:27 |
| StevenK | Haha | 05:28 |
| StevenK | File "/home/steven/launchpad/lp-sourcedeps/eggs/storm-0.19.0.99_lpwithnodatetime_r406-py2.7-linux-x86_64.egg/storm/info.py", line 95, in <genexpr> | 05:28 |
| StevenK | for attr in storm_primary) | 05:28 |
| StevenK | KeyError: 'filter' | 05:28 |
| StevenK | Yeah, we so want filter_id | 05:28 |
| StevenK | AttributeError: 'BugSubscriptionFilterImportance' object has no attribute 'id' | 05:31 |
| StevenK | Hah | 05:31 |
| StevenK | self.assertIsNot(None, bug_sub_filter_importance.id) | 05:31 |
| wgrant | Heh | 05:32 |
| wgrant | That's probably just checking it's been flushed to the store | 05:32 |
| StevenK | It then checks the filter, so it can just check that | 05:32 |
| wgrant | Ah! | 05:34 |
| wgrant | I finally found a test for apport's default private thing. | 05:34 |
| wgrant | I think there's only one. | 05:34 |
| wgrant | Hidden in a doctest | 05:34 |
| StevenK | Hah | 05:34 |
| StevenK | wgrant: I'll be pushing the fixes for that branch in a sec | 05:34 |
| wgrant | StevenK: Will look | 05:35 |
| StevenK | wgrant: Oh, pft. Fully expecting you to get distracted for 45 minutes again. :-P | 05:35 |
| wgrant | Nah | 05:35 |
| wgrant | I solved my issue | 05:35 |
| StevenK | wgrant: Diff updated. | 05:38 |
| wgrant | StevenK: Indeed, r=me | 05:38 |
| wgrant | Thanks | 05:38 |
| StevenK | I guess no-qa is probably okay | 05:40 |
| wgrant | Most certainly. | 05:40 |
| wgrant | lp-land --no-qa if you're feeling reasonably confident | 05:40 |
| StevenK | wgrant: Not like it matters. | 05:41 |
| wgrant | Heh | 05:41 |
| StevenK | It could go through ec2 twice and still land in time. | 05:41 |
| StevenK | stub: https://code.launchpad.net/~stevenk/launchpad/db-drop-id-for-bsns/+merge/117375 | 06:04 |
| StevenK | stub: And the diff for https://code.launchpad.net/~stevenk/launchpad/db-bugsubscriptionfilter-itype/+merge/117003 has been updated. | 06:30 |
| lifeless | stub: so, can you add a card for fixing this sessions issue, and liase with mthaddon on it ? | 07:33 |
| stub | lifeless: yes | 07:35 |
| czajkowski | lifeless: morning/evening any thoughts on https://bugs.launchpad.net/launchpad/+bug/1022334 https://bugs.launchpad.net/launchpad/+bug/1030584 they both seem rather similar. | 08:53 |
| _mup_ | Bug #1022334: data is not saved if launchpad has an error <bot-comment> <Apport:New> <Launchpad itself:New> <apport (Ubuntu):New> < https://launchpad.net/bugs/1022334 > | 08:53 |
| _mup_ | Bug #1030584: Make +filebug timeouts more user-friendly <Launchpad itself:Incomplete> < https://launchpad.net/bugs/1030584 > | 08:53 |
| mgz | ...back button works fine provided you have JS disabled | 08:58 |
| mgz | and I didn't think the messing around with "no I need to report a new ug" junk broke that | 08:58 |
| mgz | was (coincidentally) testing on staging the other day, which nearly always times out first time | 08:59 |
| mgz | yup, it doesn't lose your description at least | 09:02 |
| lifeless | czajkowski: the second is a confused user, with a real timeout. | 09:02 |
| mgz | you need to it 'next' again though which is lame. | 09:03 |
| lifeless | so, I don't think we should try to make form resubmital super reliable; thats a browser issue (and IME firefox preserves values just fine) | 09:04 |
| lifeless | we should fix the timeouts when submitting the bugs, and those two bugs have different symptoms | 09:04 |
| lifeless | one is a slow insert the other is a slow packaging query | 09:04 |
| lifeless | there is probably a dupe of that | 09:04 |
| czajkowski | hmm ok | 09:05 |
| mgz | lifeless: going back to "please describe the bug in a few words" does make it *look* like launchpad has lost your bug | 09:05 |
| lifeless | yes, but it hasn't | 09:05 |
| lifeless | the thing is | 09:05 |
| lifeless | its diminishing returns | 09:05 |
| mgz | even though hitting next unhides it | 09:05 |
| lifeless | say 1 in a thousand timeout on submission | 09:05 |
| czajkowski | mgz: not many people would think to hit next | 09:05 |
| mgz | this is general usability though, not just timeout related | 09:06 |
| lifeless | should we make dealing with the timeout easier, or fix the timeout ? | 09:06 |
| lifeless | mgz: welllll, its a wizard | 09:06 |
| lifeless | so that argument goes both ways | 09:06 |
| mgz | a one page wizard that resets to the start at the drop of a hat | 09:06 |
| mgz | with JS off it's much better | 09:06 |
| lifeless | mgz: I'm not sure what your point is. | 09:07 |
| mgz | because the 'search for similar bugs' and 'fill in all your details' are actually seperate | 09:07 |
| lifeless | I'm clearly not against improvements, but if I had to choose what to work on, the thing they are complaining about (not the timeout itself), isn't what I would do. | 09:07 |
| mgz | so, you can safely use back and such like without things breaking | 09:07 |
| mgz | it's like the "error when reporting a bug" issue from the other day | 09:08 |
| lifeless | yes, and that one is terrible and I think we should do better | 09:08 |
| lifeless | because its a common case not an exceptional case. | 09:08 |
| mgz | my point is it's basically the same thing, I agree for timeouts specifically fixing the timeout is more relevent | 09:09 |
| lifeless | Its similar in a very technical sense | 09:09 |
| lifeless | its not at all similar in a use case sense | 09:09 |
| lifeless | IMNSHO | 09:09 |
| mgz | so, specifically on czajkowski's question, | 09:12 |
| mgz | I'd make the first bug a usability one on using back from navigating off the +filebug page (or dupe against an existing bug on that) | 09:12 |
| mgz | and make the second one about the timeout that wgrant dug up (or dupe against similar +filebug timeout) | 09:13 |
| === matsubara is now known as matsubara-lunch | ||
| === almaisan-away is now known as al-maisan | ||
| === al-maisan is now known as almaisan-away | ||
| === matsubara-lunch is now known as matsubara | ||
| wgrant | abentley: Hi. Will you be able to QA bug #1018235 today? | 13:12 |
| _mup_ | Bug #1018235: TestRunMissingJobs.test_find_missing_ready is disabled due to spurious failures <qa-needstesting> <spurious-test-failure> <test-system> <Launchpad itself:Fix Committed by abentley> < https://launchpad.net/bugs/1018235 > | 13:12 |
| abentley | wgrant: yes. | 13:12 |
| wgrant | Great. | 13:13 |
| === rick_h_ changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 4.0*10^2 | ||
| === rick_h_ changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: rick_h | Firefighting: - | Critical bugs: 4.0*10^2 | ||
| benji | jml: I finally have some time to look at buildout for you (I've been crazy busy with interviews for the last week). I'm looking at bug 1029715 as well as digesting the comments on https://code.launchpad.net/~james-w/pkgme-service-python/buildout/+merge/116980; if there is anything else I can look at, feel free to let me know | 14:35 |
| _mup_ | Bug #1029715: bootstrap.py fails if there is a system-wide install of the desired buildout version <Buildout:New> < https://launchpad.net/bugs/1029715 > | 14:35 |
| jml | benji: thanks :) | 14:35 |
| jml | benji: we've just landed buildout for one of our leaf projects, and hope to move up the DAG today or tomorrow, | 14:36 |
| jml | benji: will let you know. very much appreciate the help. | 14:36 |
| benji | cool | 14:36 |
| benji | jml: note that I will be out for about a week starting tomorrow (moving house 1007 kilometers to the south west) so my responsiveness won't be good ;) | 14:38 |
| jml | benji: wow | 14:39 |
| benji | just one state over ;) goo.gl/maps/odLpT | 14:44 |
| jcsackett_ | sinzui: have a few minutes to chat? | 15:01 |
| === jcsackett_ is now known as jcsackett | ||
| czajkowski | jcsackett: feeling better | 15:05 |
| czajkowski | benji: free for a quick pm ? | 15:05 |
| jcsackett | czajkowski: feeling semi-functional, thanks. :-) | 15:05 |
| benji | czajkowski: sure | 15:05 |
| abentley | sinzui: I'm adding Specification.information_type, so I'm looking at patch-2209-12-*. Do you know why the column wasn't set "NOT NULL" on creation? | 15:30 |
| sinzui | abentley: we had to use a garbo job to sync existing data | 15:31 |
| abentley | sinzui: Ah! Thanks. Mine should include a default, then, I assume. | 15:32 |
| sinzui | abentley: the db rollout will only permit a schema change...no code will be released with your column add | 15:32 |
| abentley | sinzui: I know. I'm just hacking the schema for now. | 15:33 |
| sinzui | abentley: you can provide a default in the schema, but I think Lp needs an enum policy in the model, so null would be correct. | 15:35 |
| abentley | sinzui: I don't understand. We should start with all existing specs public and then we can update the model to reflect the column afterward, no? | 15:36 |
| === joey2 is now known as nv0n | ||
| === nv0n is now known as joey | ||
| sinzui | abentley: bug and branch informationtypes are governed by a policy determined by the presence of a commercial subscription and projects rules to ensure information is not accidentally disclosed. I image you want something like http://pastebin.ubuntu.com/1121640/ | 15:42 |
| sinzui | https://qastaging.launchpad.net/launchpad/+admin | 15:43 |
| abentley | sinzui: could be. I'll need to find out whether the project's "private" setting will control that. | 15:44 |
| sinzui | abentley: I am sure they will. We are adding these now because stakeholders see them as an oversight | 15:45 |
| === salgado is now known as salgado-lunch | ||
| === Ursinha` is now known as Ursinha | ||
| === beuno is now known as beuno-lunch | ||
| === beuno-lunch is now known as beuno | ||
| czajkowski | sinzui: do you think you should be able to file a bug from bugs.lp.net or should you go to the project page and file it from there, with the exception being Ubuntu ? | 16:24 |
| sinzui | czajkowski: no. | 16:24 |
| czajkowski | thats what I'd have assumed | 16:25 |
| czajkowski | but wondered did I not know about something | 16:25 |
| sinzui | The user has to find the project, find that it uses bugs, and know the bug is not already reported | 16:25 |
| === deryck is now known as deryck[lunch] | ||
| rick_h_ | jcsackett: never has a mp with just a few lines of comments given me such as headache before :P | 17:03 |
| jcsackett | that's alarming. | 17:04 |
| jcsackett | why the headache? | 17:04 |
| jcsackett | rick_h_: ^ | 17:05 |
| rick_h_ | writing up the MP, sec | 17:05 |
| rick_h_ | because I founde code doing args.callee.magic_crap and I cried for a while :P | 17:05 |
| rick_h_ | while trying to understand things | 17:06 |
| rick_h_ | jcsackett: ok, marked needs fixing, but just because I want to peek at it again. | 17:13 |
| rick_h_ | let me know if my comments don't make sense | 17:13 |
| === salgado-lunch is now known as salgado | ||
| abentley | rick_h_: have you shared that folder with me yet? | 17:38 |
| rick_h_ | abentley: ah no, sec will do right now | 17:39 |
| abentley | rick_h_: It seems not to matter what email address you use. You can even sign up for u1 in the process of accepting a share. | 17:44 |
| rick_h_ | abentley: yea, makes sense with SSO | 17:45 |
| rick_h_ | sometimes people have different accounts for work/home though so didn't get going on it, see G+ | 17:45 |
| rick_h_ | abentley: but added you, abel, huw and also shared out a G+ folder I've started to put some notes into | 17:46 |
| rick_h_ | still working on merging paper notes, etc but shared out | 17:46 |
| rick_h_ | I liked that in the June project how we had that one folder shared to go into for things | 17:46 |
| abentley | rick_h_: No, I'm saying you can accept the share based completely on the URL. It doesn't matter what account you use, and you can even create a new account. | 17:47 |
| rick_h_ | abentley: oic. | 17:48 |
| abentley | rick_h_: I'm happy to use either Google Drive&Docs or u1, but using both seems likely to cause confusion. | 17:53 |
| rick_h_ | abentley: yea, guess lack of a linux client made it hard to upload all the images to GDrive while docs is our official docs | 17:54 |
| rick_h_ | I think once we get to mockups vs the hacky wireframes the U1 will go away | 17:55 |
| rick_h_ | and it'll be real html + docs | 17:55 |
| rick_h_ | but whatever, I'm happy to play along just starting out with this and can move things as required | 17:55 |
| abentley | rick_h_: fair enough, if we assume plain files go to u1, we should be okay. | 17:55 |
| deryck | abentley, rick_h_ -- I added a few started cards on the board now. These are sort of known quantities at this point... | 18:54 |
| deryck | abentley, rick_h_ -- but I didn't attempt to get anymore detailed than just the start of our work. | 18:54 |
| rick_h_ | ok | 18:54 |
| rick_h_ | deryck: got time to chat? | 18:55 |
| deryck | I assume we're working toward getting the board accurate for next work as the week goes along. | 18:55 |
| deryck | rick_h_, sure. | 18:55 |
| rick_h_ | deryck: k, joining hangout | 18:56 |
| lifeless | o/ | 18:58 |
| abentley | lifeless: Hi. If I want to add a column and put an index on that column, do I need two branches? | 19:02 |
| lifeless | depends on the size of the table. | 19:03 |
| lifeless | If its small (thousands of rows), no. | 19:03 |
| abentley | lifeless: I'm thinking of Specification. So, yes? | 19:03 |
| lifeless | If its large > 10's of thousands of rows, yes. | 19:03 |
| abentley | lifeless: "45439 blueprints registered in Launchpad" | 19:05 |
| lifeless | yah, I think that will be safest as two patches. | 19:05 |
| lifeless | one downtime, one applied hot. | 19:05 |
| deryck | hi lifeless. I expect you'll be hearing a lot from us on Orange over the next few weeks. :) | 19:15 |
| lifeless | :) | 19:15 |
| lifeless | would you like to have a voice call at some point ? | 19:15 |
| deryck | lifeless, you know, we probably should. Maybe late this week, early next. | 19:18 |
| deryck | lifeless, give me time to get coordinated and understand as much as I can first. | 19:18 |
| lifeless | sure | 19:20 |
| abentley | lifeless: https://dev.launchpad.net/Database/LivePatching sez "All DB schema changes are landed on db-devel", but https://dev.launchpad.net/PolicyAndProcess/DatabaseSchemaChangesProcess sez, "For hot patches the target is lp:launchpad, but for cold patches it should be lp:launchpad/db-devel" | 19:36 |
| lifeless | abentley: hi | 20:00 |
| abentley | lifeless: hi. | 20:00 |
| lifeless | abentley: https://dev.launchpad.net/PolicyAndProcess/DatabaseSchemaChangesProcess is authoritative. | 20:00 |
| abentley | lifeless: Okay. But DatabaseSchemaChangesProcess says Database/LivePatching...[is] the authority [on hot-patching]. | 20:06 |
| lifeless | heh | 20:08 |
| lifeless | abentley: devel is where indexes land. See e.g. r15367 | 20:08 |
| === rick_h_ changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 4.0*10^2 | ||
| wgrant | abentley: You can't set a default when adding the column on a large table, as that causes every row to be rewritten. | 22:01 |
| wgrant | abentley: You'll need to, in one patch, create the column nullable and without a default, and then set the default. Then garbo or similar to set it. Then add the index with a live patch. | 22:02 |
| lifeless | (you can add the index before the garbo job; you probably will want to to make the garbo job efficient) | 22:06 |
| wgrant | True. | 22:08 |
| abentley | wgrant: Thanks. Sigh. | 22:43 |
| StevenK | wgrant: https://code.launchpad.net/~stevenk/launchpad/refactor-bsf/+merge/117543 | 23:12 |
| wgrant | StevenK: Only -102? You disappoint me. | 23:22 |
| StevenK | wgrant: :-( | 23:22 |
| * wallyworld finds it hard to concentrate looking across the desk at bigjools' ugly face :-( | 23:39 | |
| * bigjools turns up the music to drown out wallyworld's whinging | 23:40 | |
| StevenK | wallyworld: Given the difference in height, aren't you staring at his chest, rather than his face? | 23:40 |
| bigjools | close | 23:41 |
| wallyworld | StevenK: i was until i hoped off his lap | 23:41 |
| StevenK | wgrant: Should I add the auditor-team structsub back? | 23:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!