[00:01] StevenK: Indeed :( [00:02] StevenK: It's a bit unfortunate that Storm doesn't let us have lazy columns. [00:06] And that review is done. [00:50] wgrant: How do I declare an array col? [00:53] StevenK: List [00:53] List(type=Int()) is probably what you want [01:07] wgrant: Did you fix the search speed? [01:16] Well, whoever did, it's amazing! [01:16] huwshimi: adeuring worked on that recently [01:16] the fulltext search stuff? [01:17] rick_h_: Ah right. It's so good! [01:17] It might just be the fastest action on Launchpad... [01:23] wgrant: I have a bunch of failures because the test suite is assuming the owner can access private branches [01:28] huwshimi: Bug searches? Yeah, they're a few times faster now, it's nice. [01:28] The speed improvements are mine, the less obtuse text search is Abel's. [01:29] wgrant: Well, thanks a lot! [01:30] It's still about twice as slow as it's meant to be, but I'll hopefully work that out some time this year :) [01:32] wgrant: I didn't know we could load pages that quickly, already. [01:32] Heh [01:33] Just with the roundtrip, template render etc. [01:35] wgrant: Considering throwing in an owner clause to get_branch_privacy_filter() [01:35] StevenK: Shouldn't the owner have a grant on creation, because it has a subscription automatically? [01:36] huwshimi: hah [01:36] huwshimi: its not all doom and gloom; lots of hard work tho [01:37] huwshimi: The minimum server-side render time for a full page with auth and template rendering is around ~200-250ms. We really need to fix our stack. [01:37] wgrant: Hah, I've just had a thought -- I wonder if these test failures will die due to the sampledata changes in drop-branch-populate-aag. [01:37] And we also need slaves in the US :) [01:37] StevenK: Indeed, probably! [01:38] * StevenK wields make schema to find out [01:38] lifeless: Well, it's paying off! [01:39] huwshimi: The sharing pages are among the quickest in LP, too [01:50] wgrant: Running makeBranch(owner=owner, information_type=USERDATA) in a harness gives access_grants = {} [01:50] StevenK: I suggest working out why. [01:51] StevenK: It's possibly because it transitions the information type after creating the subscription, and the transition doesn't create grants. [01:54] wgrant: IBranchNamespace.createBranch will only subscribe if there is a BVP [01:54] Which doesn't help much for the testsuite [01:54] We can subscribe the owner if information_type is not None in the factory [01:55] Ah [01:56] Not only does it not help for the test suite, but it also ruins all of our plans. [01:58] Well, it pulls teams from the BVP and subscribes them all [01:59] StevenK: Huh? [01:59] That's not the relevant bit here. [02:00] StevenK: I've just checked, and even public branches get a default subscription for the owner. [02:00] The BVP subscription stuff is for granting others access. [02:00] branch.subscribe( [02:00] self.owner, [02:00] Right after the BVP sub. [02:00] Ah [02:00] Then why is branch.access_grants = {} :-( [02:01] if information_type is not None: [02:01] naked_branch.transitionToInformationType( [02:01] information_type, registrant, verify_policy=False) [02:01] Probably because of that. [02:02] See the if information_type in PRIVATE_INFORMATION_TYPES: [02:02] section of Bug.transitionToInformationType [02:02] It's probably missing from Branch.transitionToInformationType [02:03] wgrant: And missing_subscribers = set([who, self.owner]) ? [02:05] StevenK: That's a difficult question. [02:05] StevenK: I don't think so. [02:05] The owner certainly shouldn't be implicitly subscribed. [02:05] The actor probably should be. === jamesh__ is now known as jamesh [02:10] wgrant: Adding a missing_subscribers = set([who]) and subscribing still ends up with access_grants = {} [02:11] StevenK: Did you add the ensureAccessGrants bits? [02:31] wgrant: I have now, still {} [02:37] Woot, access_grants = {243652} [02:52] StevenK: What'd you change? [02:53] wgrant: Moved the ensureAccessGrants() section after we set information_type and call _reconcileAccess() [02:53] Ah, yes, that's fairly critical :) [04:28] wgrant: Is test_owner_member_sees_own_branches in test_branchcollection complete crack? [04:29] It creates a private branch by stacking on a existing private branch, then unsubscribes the owning team and then asserts the team owner can still see the branch. [04:29] StevenK: Right, that test no longer makes sense, as we've changed the rules. [04:29] * StevenK bins it [04:30] wgrant: The only failure left is test_initial_branches_does_not_contain_private_dev_focus_branch in code/browser/tests/test_product.py [04:34] StevenK: I would suggest making it not fail. [04:35] Obviously. [04:35] I just don't get why it does fail, I thought CIV() with no principal was anonymous [04:36] Its been replaced. [04:36] With CV() [04:37] *facepalm* [04:46] Oh, DUH [04:47] The bloody tests calls login() in the method to create the branch [04:48] wgrant: So it looks like this branch is creating a private development focus for a product and then assuming it can't be seen. [04:48] s/branch/test/ [04:48] StevenK: I got that idea from the name, yaeh. [04:49] Considering replacing the login() call with 'with person_logged_in(...)' but that may have other knock-on effects. [04:49] The bigger question is perhaps why this only makes it break now. [04:50] It calls check_permission on the dev focus, which calls visibleByUser() and I have changed that method a lot [04:51] Right. [04:51] But that suggests that the login() call is not the problem. [04:51] Or the test was buggy before. [04:51] Tossing a login(ANONYMOUS) before the call to create_initialized_view() fixes the test. [04:53] I'm not sure how this test ever worked properly. [04:57] StevenK: Ah [04:57] I just read the code [04:57] Now it is clear [04:58] StevenK: It explicitly logs in as the product owner, not the branch owner. [04:58] StevenK: Recall that product owners now have an APG by default. [04:58] Haha, which we now allow [04:58] Due to the visibleByUser() changes [04:59] Precisely. [04:59] So you can either log in as another user or remove the APG [04:59] login(ANONYMOUS) it is then [05:21] wgrant, wallyworld: https://code.launchpad.net/~stevenk/launchpad/new-branch-visibility/+merge/113484 [05:23] StevenK: Did you look into using BranchCollection to implement Branch.visibleByUser? [05:24] Not really, I didn't see how it could be a win [05:24] It removes code duplication. [05:28] wgrant: By calling IAllBranches.visibleByUser? [05:30] StevenK: BranchCollection sadly doesn't provide a means to filter by ID or URL, so it may not be doable at this stage. [05:32] wgrant: branch_filter_expressions=[Branch.id == self.id] ? [05:33] StevenK: Oh, I lie, getUtility(IAllBranches).withIds(self.id).visibleByUser(user).is_empty() should work [05:36] wgrant: http://pastebin.ubuntu.com/1075921/ ? [05:37] Hm, we can probably refactor further, and feed a list of the current branch and all branches stacked on it [05:39] Well [05:39] It's not clear whether we should preserve that behaviour at that level. [05:39] It's current behaviour at least [05:43] wgrant: http://pastebin.ubuntu.com/1075926/ [05:43] Sigh, modulo a one character typo [05:43] StevenK: That doesn't make much sense. [05:44] The visibility of a branch might depend on the branches it's stacked on. [05:44] It certainly does not depend on the branches that are stacked on it. [05:45] That was the intent [05:50] wgrant: Does http://pastebin.ubuntu.com/1075932/ make you hate me? [05:51] StevenK: That probably won't work, since it's unlikely you can check the stacked_on of a branch you don't have access to. [05:51] You probably need to preserve the original behaviour. [05:52] Just replacing _checkBranchVisibleByUser with the branchcollection thing [05:52] s/behaviour/structure/ [05:52] Yeah :-( [05:53] I suspect WithIds wants a list, though [05:54] Hmm, not sure [06:09] wgrant: Diff updated. [06:12] StevenK: r=me, but please do some data checks on prod before landing [06:13] wgrant: Such as checking all private branches have some AAGs? Or what did you have in mind? [06:13] StevenK: Actually, why not inline the branchcollection usage into visibleByUser? [06:13] Hm, good point. [06:13] _checkBranchVisibleByUser is now one statement, and it only has one callsite [06:13] Let's do that. [06:13] Might as well inline [06:13] Save another few lines. [06:13] Get you to -15 or so [06:14] -17 [06:16] -18, in fact, because I'm an idiot [06:18] wgrant: Which is fantastic, because I've wanted _checkBranchVisibleByUser dead for a while. [06:22] wgrant: What data checks did you have in mind? [06:23] StevenK: The ones I was thinking of don't really work [06:23] So let's JFDI [06:24] * StevenK tosses it at ec2 [07:41] good morning === matsubara-afk is now known as matsubara [08:41] _StringException: Text attachment: garbage [08:41] ------------ [08:41] [] [08:41] wgrant: ^ have you seen that on ec2? [08:44] StevenK: buildbot saw that twice last week. [08:44] It's spurious. [08:47] But it will mark the branch as failed, which is annoying [10:07] Any thoughts on a nice way to export the list of custom files in a PackageUpload? lp.soyuz.scripts.queue currently does: [10:07] for queue_custom in queue_item.customfiles: [10:07] self.display("\t | * %s Format: %s" [10:07] % (queue_custom.libraryfilealias.filename, [10:07] queue_custom.customformat.name)) [10:08] I exported URLs for the library files as PU.customFileUrls(), but maybe it should've been .customFiles() that returns a list of (URL, format) tuples or something ... [10:09] That might be best, indeed. [10:09] (It's on devel exactly so that we can change it ;-) ) [10:09] Yup === daker__ is now known as daker_ === daker__ is now known as daker_ === matsubara is now known as matsubara-afk [11:00] Hmm, OTOH that would mean every 'queue info' would have to make a customFiles call just to find out whether there were any [11:00] Maybe it would make more sense to fold it into getBinaryProperties [12:04] heya, I've got 3 MPs up for different workitem-related stuff [12:04] if anyone feels review-happy, there you go :) [12:11] gmb: for the addSkip thing [12:11] should it be falling back to addSuccess? [12:11] Um. [12:11] AIUI that is what testtools in 2.6 is doing [12:11] jam. A thousand botherations. Yes, it should. Good catch (and that'll teach me for self-reviewing) [12:11] otherwise, I think you break the subunit stream [12:11] in the way that it broke for 2.7 [12:12] jam, I don't - it handles the skip properly [12:12] It just doesn't call up to the parent class. [12:12] Actually, hmm.. [12:12] jam, They way it is now, it skips properly when using --subunit [12:12] it is possible that self.options.output.test_skip is doing the right output. [12:13] jam, It is... it's just not incrementing the counter on unittest.TestResult, which doesn't exist. [12:13] (For py 2.6) [12:14] gmb: ah ok. the other thing to watch out for is double-counting, but yeah, I think calling something on the super() class is useful. [12:14] gmb: we just need a # py 2.6 comment there, so we know we can nuke all that code in the future [12:14] :) [12:14] Right :) [12:14] Amusingly, the zope.testing test suite breaks horribly under 2.6. [12:14] *sigh* [12:14] But I'll go and make that change so that we fall back. [12:15] gmb: so you can't actually run the zope.testing suite in 2.6? ugh [12:15] Exactly. [12:15] You can run bits of it [12:15] But not all of it. [12:15] So "assertIn()" for example has to not be used :/ [12:18] mgz: I see a bunch more of your patches have landed. kudos [12:31] jam: I shall run the kanban update again and see if they're done-done [12:31] mgz: well, it hasn't been deployed to prod [12:31] so I think it shouldn't be [12:32] yup, Deployment::Ready is the state [12:42] jcsackett: just soyuz and translation left :) https://code.launchpad.net/~rharding/launchpad/registry_yui35/+merge/113407 [12:42] jcsackett: I tried to comment the actual changes vs moving as much as possible so it's a long MP, but itemized to help. [13:11] wgrant: https://code.launchpad.net/~cjwatson/launchpad/queue-api-custom-files/+merge/113574 ? [13:30] adeuring, https://plus.google.com/hangouts/_/040bd7afe73bdcaf379359f9eb707488709b7abd?authuser=0&hl=en [13:32] rick_h_, https://plus.google.com/hangouts/_/040bd7afe73bdcaf379359f9eb707488709b7abd?authuser=0&hl=en === jcsackett changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: jcsackett | Firefighting: - | Critical bugs: 4.0*10^2 [14:19] jcsackett, hi, I've got a couple of MPs up, can you please take a look at them on +activereviews? [14:19] jcsackett, fwiw, I am around for any questions in the next few hours [14:19] danilos: dig. i'm working through the queue. [14:19] jcsackett, cool, thanks [14:58] Did the bzr team get merged back into LP? [14:58] Or something of that sort. [14:59] yeah, of that sort. [15:14] abentley: so all three JS tests just passed in yui3.5 w/o even opening them. [15:15] rick_h_: Good news: all passed. Bad news: all = 3. :-) [15:15] hah [15:15] well three files [15:15] looks like 28 actual tests [15:17] rick_h_: Well, that's good then. === salgado is now known as salgado-lunch === matsubara-afk is now known as matsubara === salgado-lunch is now known as salgado [16:53] mpt: where would we be without you :0 [16:53] :) [16:54] jcsackett: quick one for you for a change when you get a chance: https://code.launchpad.net/~rharding/launchpad/fix_attr_quotes/+merge/113610 [16:54] In a world that was slightly more confusing but otherwise just the same [16:54] and no spam :) [17:17] jcsackett: i could use a rievew: https://code.launchpad.net/~ivo-kracht/launchpad/bug-728129/+merge/113612 [17:18] ivory: your up next as soon as i finish my current. [17:22] jcsackett: thank you, chances are that i'm not online anymore by then since it's quite late now and it's my last day of my internship. questions go to adeuring then :) [17:25] $ bzr damage [17:25] Using submit branch /home/cjwatson/src/canonical/launchpad/lp-branches/devel [17:25] Healing: 2007 lines of code [17:25] ^- I like the look of this [17:36] And I can even land that after my three queue-api-* MPs are reviewed and deployed ;-) [17:58] jcsackett: ty much for the trio of reviews today [17:58] cjwatson: LoC credit coming down the pipe? [17:58] rick_h_: stand back [17:59] cjwatson: LoC assassnator! [17:59] escept with better spelling and all that === heroux_ is now known as heroux [19:20] jcsackett: could you please do a "follow-up" review of Ivo's MP: https://code.launchpad.net/~adeuring/launchpad/bug-728129/+merge/113631 [19:21] adeuring: cool, can you verify if the test_decoratedresultset.py file is even needed? it look like it's to collect the doctest, but that doctest already exists and runs without thie test_*py file. [19:22] jcsackett: yes, we need some way to get the doc test in the tests/ directory executed. WIthout this file, the doc test is simply ignored. [19:23] jcsackett: note that there is another fle decoratedresultset.txt, but in the directory doc/, which is at oresent executed. [19:25] adeuring: dig. r=me then. [19:25] jcsackett: cool, thanks, also in Ivo's name! [20:04] lifeless: Have you seen https://dev.launchpad.net/Code/RecipeBuildDebversioning ? The more I think about it, the more it seems like a recipe build should supersede any other packaging by default, because if you don't want the recipe version, you remove the package & PPA. [20:05] I saw you make it yesterday, I think. [20:05] Your logic makes sense to me. Its complicated by apt/dpkg behaviour but not something we can really solve. [20:06] by which I mean, being a slightly higher version is a good idea. But removal is more complicated (and there are special tools to use to do that). [20:22] james_w: Could you comment on https://dev.launchpad.net/Code/RecipeBuildDebversioning ? === salgado is now known as salgado-afk === matsubara is now known as matsubara-afk === jcsackett changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 4.0*10^2 [21:52] bbiab [22:31] jcsackett: thanks for the cluster of reviews [22:32] cjwatson: you're welcome. :-) [22:32] * cjwatson runs three parallel ec2 lands, what could possibly go wrong [22:43] cjwatson: generally, that all works fine. as long as your branches weren't secretly dependent on each other. :-P [22:44] I don't *think* they were [22:44] will be a fun QA opportunity tomorrow though [23:09] wgrant, wallyworld: http://pastebin.ubuntu.com/1077299/ [23:20] FAILED (id=1, failures=662 (-3231), skips=2) [23:20] Progress!