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