[01:19] <StevenK> wgrant: Looks like there are 18 or so legitimate failures, most of them related to query counts.
[01:20] <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:23] <StevenK> wallyworld: http://pastebin.ubuntu.com/1077406/
[01:23]  * wallyworld looks
[01:26] <wallyworld> StevenK: so, you have code that i don't, but the callto getPeopleWithoutAccess in trantitionToInfoTyoe in branch is being done with people=[]
[01:27] <StevenK>             blind_subscribers = service.getPeopleWithoutAccess(
[01:27] <StevenK>                 self, self.subscribers)
[01:27] <wallyworld> so self.subscribers must be empty
[01:28] <wallyworld> or so it seems
[01:29] <StevenK>     + Subscribers: [<Person at 0x10071b50 eric (Eric the Viking)>]
[01:30] <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:31] <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:32] <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:33] <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:34] <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:35] <wgrant> It's a subselect
[01:35] <wgrant>             In(Person.id, person_ids))
[01:35] <wgrant> It has to be that line
[01:36] <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:37] <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:38] <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:39] <wallyworld> but StevenK said above there were people
     + Subscribers: [<Person at 0xeed1e90 eric (Eric the Viking)>]
     + 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:40] <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:41] <wallyworld> i guess i should have added an assert
[01:42] <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:44] <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:46] <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:47] <wgrant> But the sample data is a little anomalous.
[01:47] <StevenK> Right, so let's create a subscription
[02:15]  * wallyworld -> buy coffee
[03:40]  * mwhudson stabs the branch vocabulary
[03:57] <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:58] <lifeless> cool, let me just change rooms
[04:00] <lifeless> stub: skype or g+; just sent you a g+ invite but skype is running too ... whatever you prefer
[04:11] <StevenK> wgrant: So, query counts?
[04:13] <wgrant> StevenK: Can you paste a failure?
[04:15]  * StevenK fixes another test by use of rSP
[04:16] <StevenK> wgrant: http://pastebin.ubuntu.com/1077548/
[04:18] <wgrant> StevenK: Oh, I guess we still need the shortcircuit for public branches
[04:18] <wgrant> To avoid tripping query tests like that :)
[04:21] <StevenK> wgrant: In IBranch.visibleByUser?
[04:22] <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:23] <wgrant> StevenK: Exactly.
[04:23] <StevenK> Wow, you approving of code I wrote in anger. :-P
[04:25] <StevenK> wgrant: I'm also getting a wierd failure in lib/lp/registry/browser/tests/poll-views_0.txt
[04:25] <wgrant> Oh?
[04:26] <StevenK> wgrant: http://pastebin.ubuntu.com/1077553/
[04:33] <wgrant> StevenK: Did that fail on ec2?
[04:33] <StevenK> And locally
[04:34] <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:35] <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:36] <wgrant> It uses datetime.now() and assumes launchpad will interpret it the same way.
[04:36] <StevenK> Polls need to die horribly anyway
[04:39] <spm> StevenK: wgrant: I note you both always want features to die horribly. have you considered maybe having them be tastefully removed instead?
[04:40] <StevenK> spm: They don't deserve it.
[04:41] <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:42] <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:45] <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:51] <StevenK> wgrant: Can has look at pastebin?
[04:56] <wgrant> StevenK: Something may have been preloading subscriptions before.
[04:56] <wgrant> StevenK: Check the old query log to see what's different
[05:03] <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:04] <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:09] <StevenK> wgrant: http://pastebin.ubuntu.com/1077582/ is the test on devel, all 2500 lines of output
[05:11] <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:13] <wgrant> StevenK: Reduce the limit to 1
[05:13] <StevenK> Haha, that would do it too, I suppose
[05:14] <StevenK> wgrant: Interesting bit: http://pastebin.ubuntu.com/1077587/
[05:19] <wgrant> StevenK: Ah, I was looking at the wrong test
[05:20] <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:21] <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:22] <wallyworld> could be worse
[05:30] <spm> yeah. could be stabbed for not providing cake to deserving webops staff
[05:31] <StevenK> spm: Hahaha
[05:32] <StevenK> wgrant: So what do you think the plan is?
[05:33] <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:45] <StevenK> wgrant: Modulo being distracted by Sarah, http://pastebin.ubuntu.com/1077619/
[05:46] <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:47] <wallyworld> if i had one of those "beam me up scotty" things i could send you some
[05:49] <stub> 🎂
[05:51] <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:52] <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:53] <StevenK> Hahahaha
[05:53] <StevenK> TestProductSharingDetailsView.setUp calls login_person(self.owner)
[05:54] <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:55] <StevenK> I've not looked deeply into what that view is doing
[06:00] <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:07] <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:08] <wgrant> StevenK: http://pastebin.ubuntu.com/1077619/ shows 0 access checking queries.
[06:10] <StevenK> wgrant: What about the last one that UNIONs against BranchSubscription?
[06:13] <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:14] <wgrant> So I don't know what's going on
[06:15] <StevenK> So where is there the actual view code? I've yet to find it
[06:18] <wgrant> StevenK: lp.registry.browser.pillar
[06:19] <StevenK> WTF? That code doesn't help at all
[06:20] <wgrant> Hm?
[06:20] <wgrant> PillarPersonSharingView is what you're looking for
[06:25] <StevenK> Indeed, how does this even work?
[06:28] <StevenK> wallyworld: Maybe you could help? How is PillarPersonSharingView managing to do its work in 0 queries?
[06:29] <wallyworld> context?
[06:29] <wallyworld> when did this start happening?
[06:30] <StevenK> wallyworld: http://pastebin.ubuntu.com/1077619/ and http://pastebin.ubuntu.com/1077587/ and http://pastebin.ubuntu.com/1077556/
[06:34] <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:35] <wallyworld> ok
[06:37] <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:38] <wallyworld> StevenK: do all the other tests pass, other than the query count one?
[06:40] <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:41] <StevenK> Which makes no sense to both wgrant and I
[06:41] <wallyworld> what's your concern?
[06:42] <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:44] <wallyworld> i see that the old code has an extra person select
[06:44] <wallyworld> at the end
[06:45] <wallyworld> so where are the SP checks you are expecting?
[06:46] <wallyworld> the work to load the data is primarily done by accesspolicy stuff at the db level
[06:49] <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:50] <wallyworld> wgrant: but the current code return no branches
[06:50] <wallyworld> unless i am wrong
[06:51] <wallyworld> the initial tests were written only for bugs
[06:51] <wallyworld> since branches didn't support info type etc then
[06:52] <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:53] <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:54] <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:55] <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:56] <wgrant> But I don't see how this branch could change the set of branches that is found.
[06:57] <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:58] <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:59] <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
[07:00] <wallyworld> since initially the query count was linear
[07:00] <wallyworld> just like it appears to be now for branches
[07:01] <wgrant> Exactly. The question is how that happens, and why it stops working.
[07:02] <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:03] <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:04] <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:05] <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:08] <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:09] <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:10] <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:11] <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:12] <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:13] <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:14] <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:19] <wgrant> StevenK: So how many times is the visibleByUser called in the problematic context, in devel?
[07:19] <StevenK> wgrant: 15
[07:20] <wgrant> StevenK: And what does stepping through it show it doing?
[07:21] <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:26] <StevenK> wgrant: http://pastebin.ubuntu.com/1077708/
[07:28] <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:29] <wgrant> The project owner will be able to see them in your branch, due to the APG
[07:32] <StevenK> wgrant: self.branches in PPSV is []
[07:32] <StevenK> (for devel)
[07:33] <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:34] <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:35] <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:38] <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.
[08:08] <czajkowski> morning
[08:42] <cjwatson> Err, I just got http://paste.ubuntu.com/1077762/ from an ec2 land run - should I be concerned?
[09:49] <cjwatson> Oh, well, it didn't do it again on a retry ...
[10:29] <danilos> cjwatson, then it does mean you should be worried :)
[11:34]  * cjwatson starts to understand the layout of .../browser/ and .../templates/ and feels something else leaking out of his ears as counterbalance
[13:32] <deryck> adeuring, https://plus.google.com/hangouts/_/977b4ffbaf4a8c42a0f41a913e0632048bf13e25?authuser=0&hl=en
[13:49] <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:50] <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:51] <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:58] <jam> benji: where/when would that get set in the lp test suite?
[13:59] <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.
[14:01] <benji> jam: that might work; if you wanted to set it globally, doing it in buildout-templates/bin/test.in would make sense
[14:07] <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:16] <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:17] <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:19] <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:20] <jam> benji: I did mean register, and my pypi name is jameinel, let me double check, though.
[14:20] <jam> benji: yeah, jameinel
[14:21] <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:22] <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:23] <jam> jml: it is the version in versions.cfg :)
[14:24] <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:25] <jml> jam: interesting. I had to make changes both to launchpadlib and to lazr.restfulclient. Perhaps only one is up-to-date?
[14:26] <mpt> jml, what's "LoC neutrality"? Is it removing as many LoC as you add?
[14:26] <jml> mpt: yes.
[14:27] <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:28] <jam> the test I'm running is: lp.bugs.browser.tests.test_bugattachment_file_access.TestWebserviceAccessToBugAttachmentFiles.test_user_access_to_private_bug_attachment
[14:32] <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:33] <jml> jam: pulling latest stable
[14:34] <jml> jam: what's launchpad_for() ?
[14:35] <jml> oh launchpadlib_for
[14:36] <jam> jml: so... I can see that the value is being seen inside _browser
[14:37] <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:38] <jml> jam: also, I thought you were reporting that the env var is unset at the time of error?
[14:39] <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:40] <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:41] <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:42] <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:43] <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:44] <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:45] <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:48] <jml> forgot to switch to lxc
[15:36] <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:37] <jml> mpt: oh right. it's a short-hand for Launchpad's broader "no increased maintenance burden" policy
[15:37] <mpt> I see
[15:38] <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:47] <sinzui> Speaking of LoC, I think I need to get the knives out and cut code to help my self-esteem.
[15:51] <jml> mpt: heh, there's actually plenty of code that can be removed without affecting functionality one whit.
[15:52] <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:53] <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:57] <sinzui> mpt, huwshimi is landing such a branch now. I also pointed out out bug about the obsolete portletBody classes
[16:26] <jml> w t f.
[16:26] <jml> bzr just started thinking that my lp:divmod.org branch was a Launchpad branchh
[16:27] <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:28] <jelmer> jml: that is a bit scary..
[17:11] <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