| StevenK | wgrant: Looks like there are 18 or so legitimate failures, most of them related to query counts. | 01:19 |
|---|---|---|
| StevenK | And one very strange one where the sharing service seems to generate a completly bong query. | 01:20 |
| StevenK | wallyworld: ^ Let me get you a traceback | 01:20 |
| wallyworld | ok | 01:20 |
| StevenK | wallyworld: http://pastebin.ubuntu.com/1077406/ | 01:23 |
| * wallyworld looks | 01:23 | |
| wallyworld | StevenK: so, you have code that i don't, but the callto getPeopleWithoutAccess in trantitionToInfoTyoe in branch is being done with people=[] | 01:26 |
| StevenK | blind_subscribers = service.getPeopleWithoutAccess( | 01:27 |
| StevenK | self, self.subscribers) | 01:27 |
| wallyworld | so self.subscribers must be empty | 01:27 |
| wallyworld | or so it seems | 01:28 |
| StevenK | + Subscribers: [<Person at 0x10071b50 eric (Eric the Viking)>] | 01:29 |
| wallyworld | from the sharing service code: | 01:30 |
| wallyworld | # Find the people who can see the artifacts. | 01:30 |
| wallyworld | person_ids = [person.id for person in people] | 01:30 |
| wallyworld | the onbly way person_ids can be [] is if people is empty | 01:30 |
| wallyworld | so i'm not sure what's happening | 01:31 |
| wallyworld | can you pastebin your code? | 01:31 |
| StevenK | + Subscribers: [<Person at 0xeed1e90 eric (Eric the Viking)>] | 01:31 |
| StevenK | + person_ids = [243652] | 01:31 |
| StevenK | (It's a doctest, so pdb debugging makes me want to kill people.) | 01:32 |
| wgrant | That's probably the wrong call. | 01:32 |
| StevenK | Bah, so it is. | 01:32 |
| wallyworld | ah, i may have found it | 01:32 |
| StevenK | wallyworld: https://code.launchpad.net/~stevenk/launchpad/new-branch-visibility/+merge/113484 | 01:32 |
| wallyworld | there's also clauses in the result set In(TeamParticipation.teamID, policy_grantees) | 01:33 |
| wallyworld | maybe policy_grantees is empty | 01:33 |
| wallyworld | or artifact_grantees | 01:33 |
| wgrant | That should be a subselect, shouldn't it? | 01:34 |
| wgrant | So it won't appear as an empty sequence in the textual statement. | 01:34 |
| wallyworld | perhaps, but it doesn't appear to be coded that way | 01:34 |
| wgrant | It's a subselect | 01:35 |
| wgrant | In(Person.id, person_ids)) | 01:35 |
| wgrant | It has to be that line | 01:35 |
| wallyworld | why not this line In(TeamParticipation.teamID, policy_grantees) | 01:36 |
| StevenK | if not person_ids: return [] ? | 01:36 |
| wgrant | Yeah | 01:36 |
| wallyworld | or this one In(TeamParticipation.teamID, artifact_grantees) | 01:36 |
| wgrant | wallyworld: No. | 01:36 |
| wallyworld | they use the same construct | 01:36 |
| wgrant | That is on teamparticipation.team, and it uses a subselect | 01:36 |
| wgrant | Whereas the error is about person.id without a subselect | 01:36 |
| StevenK | wgrant, wallyworld: http://pastebin.ubuntu.com/1077416/ is the full statement | 01:37 |
| wgrant | launchpad_dev=# SELECT COUNT(*) FROM branchsubscription WHERE branch = (SELECT id FROM branch WHERE unique_name='~name12/firefox/main'); | 01:37 |
| wallyworld | yes, i have read the code again and person ids doesn't use the sub select | 01:37 |
| wgrant | count | 01:37 |
| wgrant | ------- | 01:37 |
| wgrant | 0 | 01:37 |
| wgrant | StevenK: There is the problem | 01:37 |
| wgrant | (1 row) | 01:37 |
| wgrant | StevenK: So you'll want to fix the sharing service to not crash when given no people, but you might also need to fix sampledata. | 01:38 |
| wallyworld | i don't think the service should be called with no people | 01:38 |
| wallyworld | if no people, don't call it | 01:38 |
| wallyworld | but StevenK said above there were people | 01:39 |
| wallyworld | <StevenK> + Subscribers: [<Person at 0xeed1e90 eric (Eric the Viking)>] | 01:39 |
| wallyworld | <StevenK> + person_ids = [243652] | 01:39 |
| StevenK | Different call, as wgrant surmised | 01:39 |
| wgrant | When a statement from a human is in conflict with a traceback, disregard the human statement :) | 01:39 |
| wallyworld | ok. i believed what i was told :-) | 01:40 |
| StevenK | authorised_people = [self.branch.owner] | 01:40 |
| StevenK | Unauthorized: (<Branch u'~name12/firefox/main' (1)>, 'owner', 'launchpad.View') | 01:40 |
| wallyworld | so, the design of getPeopleWithoutAccess was to be called with a non empty list | 01:40 |
| * StevenK stabs things | 01:40 | |
| wallyworld | i guess i should have added an assert | 01:41 |
| StevenK | Right, it changes the information type, reconciles access, tries to render Branch:+index, and fails hard because no one has access to the branch. | 01:42 |
| wallyworld | that also happended at some point with bugs | 01:44 |
| wallyworld | i think reassigning a private bug | 01:44 |
| wallyworld | i can't recall the solution, perhaps redirect to the project page with a message i think it was | 01:44 |
| StevenK | wgrant: The branch in question is PUBLIC, I should create a branchsubscription for the owner anyway? | 01:46 |
| wgrant | StevenK: All new branches have a subscription created for the owner. | 01:46 |
| wgrant | They can remove it, of course. | 01:46 |
| wgrant | But the sample data is a little anomalous. | 01:47 |
| StevenK | Right, so let's create a subscription | 01:47 |
| * wallyworld -> buy coffee | 02:15 | |
| * mwhudson stabs the branch vocabulary | 03:40 | |
| lifeless | stub: hi | 03:57 |
| lifeless | stub: can we catch up soon ? [after morning cofee :)] | 03:57 |
| stub | lifeless: hi | 03:57 |
| stub | lifeless: Coffeed up already, been here a while; just offline on IRC | 03:57 |
| lifeless | cool, let me just change rooms | 03:58 |
| lifeless | stub: skype or g+; just sent you a g+ invite but skype is running too ... whatever you prefer | 04:00 |
| StevenK | wgrant: So, query counts? | 04:11 |
| wgrant | StevenK: Can you paste a failure? | 04:13 |
| * StevenK fixes another test by use of rSP | 04:15 | |
| StevenK | wgrant: http://pastebin.ubuntu.com/1077548/ | 04:16 |
| wgrant | StevenK: Oh, I guess we still need the shortcircuit for public branches | 04:18 |
| wgrant | To avoid tripping query tests like that :) | 04:18 |
| StevenK | wgrant: In IBranch.visibleByUser? | 04:21 |
| StevenK | Hm, I wonder if this evil hack fixes it | 04:22 |
| wgrant | StevenK: Yeah | 04:22 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1077551/ | 04:22 |
| wgrant | StevenK: Exactly. | 04:23 |
| StevenK | Wow, you approving of code I wrote in anger. :-P | 04:23 |
| StevenK | wgrant: I'm also getting a wierd failure in lib/lp/registry/browser/tests/poll-views_0.txt | 04:25 |
| wgrant | Oh? | 04:25 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1077553/ | 04:26 |
| wgrant | StevenK: Did that fail on ec2? | 04:33 |
| StevenK | And locally | 04:33 |
| wgrant | StevenK: I don't care about locally. ec2 is all that's interesting :) | 04:34 |
| StevenK | Oh, hah. It failed on ec2 with [<storm.databases.postgres.PostgresConnection object at 0x168662d0>] | 04:34 |
| wgrant | Yeah | 04:34 |
| * StevenK ignores it at spurious 2.7 garbage | 04:34 | |
| StevenK | s/at/as/ | 04:34 |
| wgrant | Nah | 04:34 |
| StevenK | wgrant: And thoughts on http://pastebin.ubuntu.com/1077556/ ? | 04:35 |
| wgrant | StevenK: It's because of the local timezone. | 04:35 |
| StevenK | Haha | 04:35 |
| StevenK | Polls fail | 04:35 |
| wgrant | Test fail | 04:35 |
| wgrant | It uses datetime.now() and assumes launchpad will interpret it the same way. | 04:36 |
| StevenK | Polls need to die horribly anyway | 04:36 |
| spm | StevenK: wgrant: I note you both always want features to die horribly. have you considered maybe having them be tastefully removed instead? | 04:39 |
| StevenK | spm: They don't deserve it. | 04:40 |
| spm | that wasn't my implication | 04:41 |
| spm | I might have been alluding to deep and disturbing psychological problems that are currently assailing two of my close colleagues. might. | 04:41 |
| spm | there's probably room for a "trolling" in that description too | 04:42 |
| StevenK | spm: Polls are special. They have been removed once, and then it turned out a few community teams required them to elect new members, so they were added back. | 04:42 |
| spm | lovely | 04:42 |
| StevenK | spm: You're not implying that wgrant and I are closet psychopaths? :-) | 04:45 |
| spm | sure I was | 04:45 |
| wgrant | Closet!? | 04:45 |
| spm | point | 04:45 |
| spm | it could be a very BIG closet | 04:45 |
| StevenK | wgrant: Can has look at pastebin? | 04:51 |
| wgrant | StevenK: Something may have been preloading subscriptions before. | 04:56 |
| wgrant | StevenK: Check the old query log to see what's different | 04:56 |
| StevenK | wgrant: bin/test -m test_pillar_sharing -t test_view_query_count still runs four tests, can I nail it down further? | 05:03 |
| wgrant | StevenK: -t specifies a regex | 05:03 |
| wgrant | Multiple regexes are ORed | 05:03 |
| wgrant | So -t test_pillar_sharing.*test_view_query_count might work | 05:04 |
| wgrant | Otherwise -t test_pillar_sharing.TestProductSharingDetailsView.test_view_query_count should be pretty specific | 05:04 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1077582/ is the test on devel, all 2500 lines of output | 05:09 |
| wgrant | StevenK: I was more interested in just the interesting bit. | 05:11 |
| StevenK | Yeah, I'm just looking how to get the statement recorder to print out what it saw. | 05:11 |
| wgrant | StevenK: Reduce the limit to 1 | 05:13 |
| StevenK | Haha, that would do it too, I suppose | 05:13 |
| StevenK | wgrant: Interesting bit: http://pastebin.ubuntu.com/1077587/ | 05:14 |
| wgrant | StevenK: Ah, I was looking at the wrong test | 05:19 |
| wgrant | StevenK: Given that it's viewing +sharing, it's probably logged in as the product owner | 05:20 |
| wgrant | StevenK: And you'll never guess who owns the branches that it creates. | 05:20 |
| StevenK | Hahah | 05:20 |
| wgrant | So the test is buggy | 05:20 |
| wgrant | It verifies that the query count is constant, but only in a very exceptional case. | 05:20 |
| StevenK | def makeArtifactGrantee( | 05:21 |
| StevenK | self, grantee=None, with_bug=True, | 05:21 |
| StevenK | with_branch=False, security=False): | 05:21 |
| * StevenK stabs wallyworld | 05:21 | |
| wallyworld | why? | 05:21 |
| StevenK | For crimes against PEP8 | 05:21 |
| wallyworld | could be worse | 05:22 |
| spm | yeah. could be stabbed for not providing cake to deserving webops staff | 05:30 |
| StevenK | spm: Hahaha | 05:31 |
| StevenK | wgrant: So what do you think the plan is? | 05:32 |
| wgrant | StevenK: Well, see what the query count is if the owner is someone else. I suspect it'll be the same with the new and old code. | 05:33 |
| wgrant | StevenK: That's an important bug, but not a regression. | 05:33 |
| StevenK | wgrant: Modulo being distracted by Sarah, http://pastebin.ubuntu.com/1077619/ | 05:45 |
| wallyworld | spm: but i gave you some cake | 05:46 |
| spm | yeah. could be stabbed for not providing non-virtual cake to deserving webops staff | 05:46 |
| spm | fixed. | 05:46 |
| wallyworld | if i had one of those "beam me up scotty" things i could send you some | 05:47 |
| stub | 🎂 | 05:49 |
| StevenK | wallyworld: Money can be exchanged for goods, such as cake, and services, such as cake delivery to Canberra. | 05:51 |
| wallyworld | StevenK: ssshhh | 05:51 |
| wgrant | StevenK: I wonder if the user might be an admin, then. | 05:52 |
| wgrant | StevenK: It doesn't explicitly log in, AFAICT | 05:52 |
| wgrant | So it might be running as some user tha happens to leak from the factory. | 05:52 |
| StevenK | Hahahaha | 05:53 |
| StevenK | TestProductSharingDetailsView.setUp calls login_person(self.owner) | 05:53 |
| wgrant | StevenK: Right, that's what I expected. But then I wonder why the test works even after you change the branch owner. | 05:54 |
| StevenK | Well, it's only a query count | 05:54 |
| StevenK | I've not looked deeply into what that view is doing | 05:55 |
| wgrant | StevenK: Well, it's somehow doing an access check in 0 queries. | 06:00 |
| StevenK | wgrant: Shall we just bump the test from 12 to 26 and move on for the time being? | 06:00 |
| wgrant | StevenK: No. You need to work out why there are 0 queries initially. | 06:07 |
| StevenK | There can't be zero. Is no understand | 06:07 |
| wgrant | StevenK: http://pastebin.ubuntu.com/1077619/ shows 0 access checking queries. | 06:08 |
| StevenK | wgrant: What about the last one that UNIONs against BranchSubscription? | 06:10 |
| wgrant | StevenK: Right, it only finds visible branches, same as with your changes, but then it doesn't do any subsequent checks. | 06:13 |
| wgrant | I can't see any code to prefill the authorization cache. | 06:13 |
| wgrant | So I don't know what's going on | 06:14 |
| StevenK | So where is there the actual view code? I've yet to find it | 06:15 |
| wgrant | StevenK: lp.registry.browser.pillar | 06:18 |
| StevenK | WTF? That code doesn't help at all | 06:19 |
| wgrant | Hm? | 06:20 |
| wgrant | PillarPersonSharingView is what you're looking for | 06:20 |
| StevenK | Indeed, how does this even work? | 06:25 |
| StevenK | wallyworld: Maybe you could help? How is PillarPersonSharingView managing to do its work in 0 queries? | 06:28 |
| wallyworld | context? | 06:29 |
| wallyworld | when did this start happening? | 06:29 |
| StevenK | wallyworld: http://pastebin.ubuntu.com/1077619/ and http://pastebin.ubuntu.com/1077587/ and http://pastebin.ubuntu.com/1077556/ | 06:30 |
| === matsubara-afk is now known as matsubara | ||
| wallyworld | StevenK: the one with 25 queries, what changes were done for that? | 06:34 |
| StevenK | wallyworld: That branch is my new-branch-visibility branch that uses APG and AAG to determine branch access rather than subscriptions | 06:34 |
| StevenK | wallyworld: The other two pastebins are against devel | 06:34 |
| wallyworld | ok | 06:35 |
| wallyworld | so the question you want to know i think is why changing the branch owner to some other person other than the pillar owner reduces the query count | 06:37 |
| wallyworld | StevenK: do all the other tests pass, other than the query count one? | 06:38 |
| StevenK | wallyworld: bin/test -vvt test_pillar_sharing == Ran 40 tests with 1 failures and 0 errors in 1 minutes 4.142 seconds. | 06:40 |
| wallyworld | StevenK: so looking at the devel branch with owner=someperson, the query count has just been reduced by 1, no? | 06:40 |
| wallyworld | ie down to 10 | 06:40 |
| StevenK | Which makes no sense to both wgrant and I | 06:41 |
| wallyworld | what's your concern? | 06:41 |
| wallyworld | (save me trying to grok the sql) | 06:42 |
| wgrant | The issue is not that changing the owner reduces the query count. | 06:42 |
| wgrant | It's that it doesn't increase it. | 06:42 |
| wgrant | Somehow the old code is taking 0 queries to do security proxy access checks. | 06:42 |
| wgrant | The new code is doing one per branch. | 06:42 |
| wallyworld | i see that the old code has an extra person select | 06:44 |
| wallyworld | at the end | 06:44 |
| wallyworld | so where are the SP checks you are expecting? | 06:45 |
| wallyworld | the work to load the data is primarily done by accesspolicy stuff at the db level | 06:46 |
| StevenK | wallyworld: Where at the DB level? | 06:49 |
| wgrant | wallyworld: It's not the DB level that's the problem | 06:49 |
| wgrant | It fetches the branches from the DB, then the securityproxies need to verify launchpad.View | 06:49 |
| wgrant | In devel at present the launchpad.View checks take no queries | 06:49 |
| wallyworld | StevenK: in the queries performed by findArtifactsByGrantee | 06:49 |
| wallyworld | wgrant: but the current code return no branches | 06:50 |
| wallyworld | unless i am wrong | 06:50 |
| wallyworld | the initial tests were written only for bugs | 06:51 |
| wallyworld | since branches didn't support info type etc then | 06:51 |
| wgrant | wallyworld: That's actually not a crazy thought. | 06:52 |
| wgrant | Except that this should have broken last week when we started adding AAGs. | 06:52 |
| wallyworld | why? | 06:53 |
| wallyworld | if the tests didn't create any branches | 06:53 |
| wallyworld | then nothing would have broken | 06:53 |
| wgrant | Oh, did the test change? | 06:53 |
| wallyworld | not afaik. | 06:53 |
| wallyworld | the test never created any branches to check | 06:54 |
| wallyworld | i think | 06:54 |
| wgrant | Does so. | 06:54 |
| wallyworld | there's a bunch of test modules that need to have extra tests for branhces added | 06:54 |
| wgrant | def makeArtifactGrantee( | 06:54 |
| wgrant | self, grantee=None, with_bug=True, | 06:54 |
| wgrant | with_branch=False, security=False): | 06:54 |
| wgrant | for x in range(0, 15): | 06:54 |
| wgrant | self.makeArtifactGrantee(person, True, True, False) | 06:54 |
| StevenK | So it makes 15 bugs and branches | 06:55 |
| StevenK | And then dies when the query count increases by 15. | 06:55 |
| wallyworld | but are any of those branches included in the results | 06:55 |
| wgrant | Not until last week | 06:55 |
| wallyworld | right ok | 06:55 |
| wgrant | But I don't see how this branch could change the set of branches that is found. | 06:56 |
| StevenK | But it doesn't? The queries that are added are all access checks | 06:57 |
| StevenK | If Branch and BranchSubscription are cached by earlier queries which sounds likely, the query count would be low | 06:57 |
| wallyworld | i'm still not sure why we expect changing the owner will change the results returned | 06:57 |
| wgrant | StevenK: BranchSubscription doesn't appear to be cached. | 06:57 |
| wgrant | It's not even cachable. | 06:57 |
| wgrant | StevenK: It's used as part of the query's internal access check, but it's not returned. | 06:58 |
| wgrant | wallyworld: I was expecting that we were hitting the branch owner short-circuit. | 06:58 |
| wgrant | But StevenK reports that the query count remains low even when a non-owner is used. | 06:58 |
| wallyworld | well it dropped from 11 to 10 | 06:58 |
| StevenK | Yes, defined as 'remains low' | 06:59 |
| StevenK | Rather than 26 | 06:59 |
| wallyworld | so with bugs, they are bulk loaded | 06:59 |
| wallyworld | not sure about branches | 06:59 |
| wgrant | Right, bug searches generally automatically populate the access cache. | 06:59 |
| wallyworld | that's why this query count test was added | 06:59 |
| wallyworld | since initially the query count was linear | 07:00 |
| wallyworld | just like it appears to be now for branches | 07:00 |
| wgrant | Exactly. The question is how that happens, and why it stops working. | 07:01 |
| wgrant | I suspect there isn't actually any prepopulation of the access cache. We're just hitting another short-circuit somewhere. | 07:02 |
| wgrant | So the test is buggy. | 07:02 |
| wallyworld | why is the test buggy? | 07:03 |
| wallyworld | it verifies that the query count is independent of nr of bugs | 07:03 |
| wgrant | It's asserting query count linearity, but it looks like it's only linear in a special case that the test hits, rather than the general case. | 07:03 |
| wallyworld | it used to fail though before changes were made | 07:04 |
| wallyworld | to fix it | 07:04 |
| wallyworld | fix the code i mean | 07:04 |
| wallyworld | i can't recall what was done though | 07:04 |
| wallyworld | it's not asserting linearity, it's ensuring there is no linearity | 07:05 |
| wgrant | Er, yeah, that. | 07:05 |
| wgrant | Sub-linearity :) | 07:05 |
| wallyworld | well, the intent is that the query count doesn't increase as the number of artifacts increases | 07:08 |
| wallyworld | who is to say it can't decrease by 1 or 2 | 07:08 |
| wallyworld | depending on what the exact data is | 07:08 |
| wgrant | wallyworld: That's not the issue | 07:08 |
| wgrant | That issue is that somehow the old code is not issuing extra queries | 07:08 |
| wgrant | The new code is | 07:08 |
| wgrant | The new code needs to not | 07:08 |
| wgrant | But we can't work out how the old code manages to not | 07:09 |
| wallyworld | so fix the new code :-) | 07:09 |
| wgrant | So we can't make the new code not. | 07:09 |
| wgrant | StevenK: Breaking on visibleByUser in the old code didn't reveal anything useful? | 07:09 |
| wallyworld | wgrant: i'm not sure how did old code did its thing, too long ag | 07:09 |
| wallyworld | ago | 07:09 |
| wgrant | Indeed, I didn't really expect you to remember :) | 07:10 |
| wallyworld | wgrant: bugtasks = list(getUtility(IBugTaskSet).search(param)) | 07:10 |
| wallyworld | that's what the old code does to load the bugs | 07:10 |
| wallyworld | where param has the bug ids | 07:11 |
| wallyworld | param = BugTaskSearchParams(user=user, bug=any(*bug_ids)) | 07:11 |
| wallyworld | iso it's whatever IBugTaskSet does | 07:11 |
| wgrant | Yes, bugtaskset precaches authorization | 07:11 |
| wgrant | But branchcollection appears not to | 07:11 |
| wallyworld | yes | 07:12 |
| wallyworld | so isn't that the answer? | 07:12 |
| wallyworld | the old code for branches | 07:12 |
| wallyworld | all_branches = getUtility(IAllBranches) | 07:12 |
| wallyworld | wanted_branches = all_branches.visibleByUser(user).withIds( | 07:12 |
| wallyworld | *branch_ids) | 07:12 |
| wallyworld | branches = list(wanted_branches.getBranches()) | 07:12 |
| wgrant | Hmmm? | 07:13 |
| wgrant | The answer to how it avoids queries is not "it's not precached" | 07:13 |
| wgrant | That's the anti-answer :) | 07:13 |
| wallyworld | no, i meran bugtaskset precahces, so eliminates queries | 07:13 |
| wallyworld | branch collection does not | 07:13 |
| wallyworld | so queries increase | 07:13 |
| StevenK | So, I think the 10th query in the devel test is IBranchCollection.visibleByUser() | 07:14 |
| wgrant | But *something* manages to avoid an access query for the 15 branches in the old code! | 07:14 |
| wgrant | The key thing is to identify what that is. | 07:14 |
| StevenK | I'm not certain why it is called 15 times, but only generates one query | 07:14 |
| StevenK | Well, I'm certain why it's called | 07:14 |
| wallyworld | so it must be in branchcollection | 07:14 |
| StevenK | branchcollection gives me a headache | 07:14 |
| StevenK | It's horrible | 07:14 |
| wallyworld | yep | 07:14 |
| wgrant | StevenK: So how many times is the visibleByUser called in the problematic context, in devel? | 07:19 |
| StevenK | wgrant: 15 | 07:19 |
| wgrant | StevenK: And what does stepping through it show it doing? | 07:20 |
| StevenK | wgrant: Hold on a moment, visibleByUser returns a collection | 07:21 |
| StevenK | So it probably returns all fifteen in one query | 07:21 |
| wgrant | StevenK: I mean Branch.visibleByUser, not *BranchCollection.visibleByUser | 07:21 |
| StevenK | Branch.visibleByUser is not called | 07:21 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1077708/ | 07:26 |
| wgrant | StevenK: Hm | 07:28 |
| wgrant | StevenK: Of course, the alternate explanation is that the user can't actually see any of the branches in the devel test. | 07:28 |
| wgrant | StevenK: So there's nothing returned to check access on. | 07:28 |
| wgrant | StevenK: Can you check that hypothesis? | 07:28 |
| wgrant | The project owner will be able to see them in your branch, due to the APG | 07:29 |
| StevenK | wgrant: self.branches in PPSV is [] | 07:32 |
| StevenK | (for devel) | 07:32 |
| wgrant | StevenK: Ha ha hahhah | 07:33 |
| wgrant | That explains it. | 07:33 |
| StevenK | [<Branch u'~person-name-100003/product-name-100004/branch-100010' (77)>, ...] for my branch | 07:33 |
| StevenK | So it caches nothing and was a useless test | 07:34 |
| wgrant | It's good for bugs. | 07:34 |
| wgrant | Just not branches. | 07:34 |
| wallyworld | it was designed primarily for bugs | 07:34 |
| wallyworld | branches weren't done yet | 07:34 |
| StevenK | So it's awesome that we've wasted a few hours bashing our head against it | 07:34 |
| wgrant | Which is unsurprising, given that branches didn't know about the access schema when it was written | 07:34 |
| StevenK | wgrant: So, 12 -> 26 and move on? Please? | 07:35 |
| wgrant | StevenK: Do file a critical bug about it, but yes. | 07:35 |
| adeuring | good morning | 07:35 |
| StevenK | wgrant: https://bugs.launchpad.net/launchpad/+bug/1021622 | 07:38 |
| _mup_ | Bug #1021622: PillarPersonSharingView uses O(n) queries for branch access <disclosure> <Launchpad itself:Triaged> < https://launchpad.net/bugs/1021622 > | 07:38 |
| wgrant | StevenK: Thanks. | 07:38 |
| czajkowski | morning | 08:08 |
| === almaisan-away is now known as al-maisan | ||
| cjwatson | Err, I just got http://paste.ubuntu.com/1077762/ from an ec2 land run - should I be concerned? | 08:42 |
| === jamestunnicliff_ is now known as jamestunnicliffe | ||
| === adeuring changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: adeuring | Firefighting: - | Critical bugs: 4.0*10^2 | ||
| cjwatson | Oh, well, it didn't do it again on a retry ... | 09:49 |
| danilos | cjwatson, then it does mean you should be worried :) | 10:29 |
| === al-maisan is now known as almaisan-away | ||
| === matsubara is now known as matsubara-afk | ||
| * cjwatson starts to understand the layout of .../browser/ and .../templates/ and feels something else leaking out of his ears as counterbalance | 11:34 | |
| deryck | adeuring, https://plus.google.com/hangouts/_/977b4ffbaf4a8c42a0f41a913e0632048bf13e25?authuser=0&hl=en | 13:32 |
| jam | benji: you landed some stuff in the lazr code (and launchpadlib) that seem to be dealing with httplib2 0.7+ (which enables ssl validation by default). | 13:49 |
| jam | I'm trying to land some of that in launchpad itself (because I want a patch to launchpadlib's test suite to pass on py2.7) | 13:49 |
| jam | however, httplib 0.7 breaks launchpad's test suite because of the ssl validation. | 13:50 |
| jam | is it sane to just set the env var for the test suite/ | 13:50 |
| jam | ? | 13:50 |
| benji | jam: I'm trying to remember that branch... oh, I think that is the one by jml; yeah, setting the env. variable for the test suite would work. | 13:51 |
| jam | benji: where/when would that get set in the lp test suite? | 13:58 |
| benji | jam: hmm, let me look | 13:59 |
| jam | maybe in a layer? | 13:59 |
| jam | LPFunctionalLayer is the one the specific test I'm looking at right now. | 13:59 |
| benji | jam: that might work; if you wanted to set it globally, doing it in buildout-templates/bin/test.in would make sense | 14:01 |
| cjwatson | What's a sensible way to write a test that attempts to open a URL on the appserver that traverses to one on the librarian? | 14:07 |
| cjwatson | I can use setupBrowserForUser to get something that knows how to open URLs on the appserver, and urlopen to open librarian files, but I can't seem to find something that works for both | 14:07 |
| cjwatson | (I'm in LaunchpadFunctionalLayer) | 14:07 |
| jam | benji, bac, jcsackett: I just committed and uploaded launchpadlib-0.10.2 (as approved). Can you upload it to pypi? I can do everything but that step. | 14:16 |
| jml | jam, benji: let me know if I can help. | 14:16 |
| benji | jam: if you're going to be making releases, then you should have pypi access; give me your pypi user name and I'll give you access | 14:17 |
| benji | jam: to clarify, we don't upload releases to pypi, we just register them (although, we should upload them in my opinion, but that's a seperate issue) | 14:19 |
| jam | benji: I did mean register, and my pypi name is jameinel, let me double check, though. | 14:20 |
| jam | benji: yeah, jameinel | 14:20 |
| benji | jam: you're all set | 14:21 |
| jam | jml: so the patch you requested ends up *requiring* httplib2 0.7+ because it is passing a parameter that doesn't exist in 0.6 | 14:21 |
| jam | however, using 0.7 means that the test suite aborts all over the place because by default the ssl certificate is checked and fails | 14:21 |
| jam | I'm guessing we want to push forward, and fix that, rather than changing the patch to be backward compatible? | 14:22 |
| jml | jam: I think so. 0.6 isn't even the version in lucid. | 14:22 |
| jam | jml: it is the version in versions.cfg :) | 14:23 |
| jam | jml: the problem right now is that I set the env var in bin/test, and if I drop into pdb I see it set, but the test still fails | 14:24 |
| jml | jam: interesting. I had to make changes both to launchpadlib and to lazr.restfulclient. Perhaps only one is up-to-date? | 14:25 |
| mpt | jml, what's "LoC neutrality"? Is it removing as many LoC as you add? | 14:26 |
| jml | mpt: yes. | 14:26 |
| jam | jml: something is stripping the env variable in "launchpad_for() | 14:27 |
| jml | jam: remind me which code-base that function is in? | 14:27 |
| jam | as in, it is set, I break on _request and then it is no longer set | 14:27 |
| jam | lazr.restfulclient._browser.Browser._request is the chain that ends up failing | 14:27 |
| jam | which you updated to support the flag | 14:27 |
| jam | the test I'm running is: lp.bugs.browser.tests.test_bugattachment_file_access.TestWebserviceAccessToBugAttachmentFiles.test_user_access_to_private_bug_attachment | 14:28 |
| jam | I'm sure I'm running updated code, as I'm getting the error: SSLHandshakeError: [Errno 1] _ssl.c:504: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed: perhaps set LP_DISABLE_SSL_CERTIFICATE_VALIDATION if certificate validation failed and you genuinely trust the remote server. | 14:32 |
| jam | which means it asks me to set the variable I have set. | 14:32 |
| jml | jam: pulling latest stable | 14:33 |
| jml | jam: what's launchpad_for() ? | 14:34 |
| jml | oh launchpadlib_for | 14:35 |
| jam | jml: so... I can see that the value is being seen inside _browser | 14:36 |
| jam | self.disable_ssl_certificate_validation is True | 14:37 |
| jam | and I'm still getting an SSL handshake failure... | 14:37 |
| jml | jam: perhaps I missed a place in my patch? can you paste the full stack? | 14:37 |
| jml | jam: also, I thought you were reporting that the env var is unset at the time of error? | 14:38 |
| jam | jml: lp:///~jameinel/launchpad/py27-introduction-1020667 | 14:39 |
| jam | jml: I thought that was the problem, as I was still getting the error. | 14:39 |
| jml | jam: ah right. | 14:39 |
| jam | But no, it is set, and tracking down I see RestfulHttp.__init__ | 14:40 |
| jam | sees the disabled flag correctly | 14:40 |
| jam | and after super() | 14:40 |
| jam | super().__init__() | 14:40 |
| jam | self.disable_ssl_certificate_validation is True | 14:40 |
| jml | then my bet is that there's a request I missed | 14:41 |
| jml | in the absence of more information, like a stack trace. (which I'll get soon, once I up download-cache, make schema and run the test) | 14:41 |
| jam | jml: http://pastebin.ubuntu.com/1078135/ | 14:41 |
| jam | jml: you'll need to update download-cache as well | 14:42 |
| jam | once I actually commit the new packages | 14:42 |
| jam | done | 14:42 |
| jam | rev 498 of download-cache | 14:42 |
| jam | anyway, I'm at my EOD here, so if you want to poke at it, that would be great. | 14:43 |
| jam | but it can wait until monday as well. | 14:43 |
| jam | jml: (this is all just fallout of trying to get a small 2-line patch to make introduction.txt not fail when run under python-2.7, so I'm trying to avoid fixing-the-world just to get that changed. But I want to do the 'right' thing as well) | 14:44 |
| jml | jam: I can see how this is annoying. | 14:44 |
| jam | anyway, enjoy your weekend | 14:45 |
| jml | jam: you too | 14:45 |
| cjwatson | Maybe the correct answer (to my question above) is to manually follow the first redirect to the librarian. | 14:45 |
| jml | forgot to switch to lxc | 14:48 |
| jml | mpt: what prompted that? | 15:36 |
| mpt | jml, just saw you mention it in a mailing list message, and I hadn't seen it mentioned before, that's all | 15:36 |
| jml | mpt: oh right. it's a short-hand for Launchpad's broader "no increased maintenance burden" policy | 15:37 |
| mpt | I see | 15:37 |
| mpt | If you want a list of features to remove to make up the numbers... ;-) | 15:38 |
| czajkowski | mpt: only if we can submit the same amount for you to remove elsewhere on design :) | 15:38 |
| sinzui | Speaking of LoC, I think I need to get the knives out and cut code to help my self-esteem. | 15:47 |
| jml | mpt: heh, there's actually plenty of code that can be removed without affecting functionality one whit. | 15:51 |
| mpt | czajkowski, there are almost certainly ways to make Launchpad prettier by reducing the amount of CSS. Would that count? | 15:52 |
| jml | yeah. | 15:52 |
| czajkowski | mpt: huw is working on that atm when he's not working on other areas, he's been fixing the layout of side bars and this week fixed links on the blog | 15:53 |
| mpt | grand | 15:53 |
| sinzui | mpt, huwshimi is landing such a branch now. I also pointed out out bug about the obsolete portletBody classes | 15:57 |
| === matsubara-afk is now known as matsubara | ||
| === salgado is now known as salgado-lunch | ||
| jml | w t f. | 16:26 |
| jml | bzr just started thinking that my lp:divmod.org branch was a Launchpad branchh | 16:26 |
| jelmer | jml: how do you mean, aren't all lp:... branches Launchpad branches? | 16:27 |
| jml | jelmer: as in a branch of lp:launchpad | 16:27 |
| jml | jelmer: the .bzr/branch/location was a URL to the branch of jam's I just checked out. | 16:27 |
| jml | jelmer: I'm guess pebkac, but ... wow. | 16:27 |
| jelmer | jml: that is a bit scary.. | 16:28 |
| === salgado-lunch is now known as salgado | ||
| cjwatson | I'd love a review of https://code.launchpad.net/~cjwatson/launchpad/queue-api-fix-urls/+merge/113776, if anyone's still around; nearing the end of this arc, and if I can land it today then I can QA it over the weekend and hopefully delete lots of code on Monday | 17:11 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!