[12:02] <jtv> Do we still require the backquotes for """See IThisClass.""" docstrings?
[12:13] <noodles775> It's still listed here: https://dev.launchpad.net/PythonStyleGuide#Docstrings
[12:22] <jtv> noodles775: Thanks!  Then that settles it for me, and I'll continue to be a prick about it.  :-)
[12:28] <noodles775> heh... I meant that if there's no reason to use it, update the wiki (and bring it up for the review meeting?). I assume it's used for pydoctor, but have never run our code through pydoctor myself.
[15:32] <leonardr> abentley, can i trouble you to review https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/server-takes-precedence/+merge/28059 ?
[15:32] <abentley> leonardr, sure.
[15:33] <abentley> leonardr, why are 'team' and 'person' different types?
[15:35] <leonardr> abentley: 'team' has some features like membership_policy that 'person' doesn't have
[15:35] <leonardr> same reason ITeam and IPerson are different interfaces
[15:43] <abentley> leonardr, it seems like this behaviour should require that the returned type is derived from the expected type, don't you think?
[15:55] <leonardr> abentley: i don't think so--i think if the client and the server disagree for any reason, the client is just wrong. in real life, the only reason they'll disagree is because of an inheritance situation
[15:56] <leonardr> but there's no way to represent inheritance in wadl, so we can't be much more sophisticated
[15:57] <abentley> leonardr, I don't think it matters who's wrong.  If the returned object's interface isn't a superset of the expected interface, chaos will result.
[16:00] <abentley> leonardr, I am having trouble finding out where the 'people' collection is defined, but PersonSet always returns IPerson, not ITeam, which seems to contradict your documentation.
[16:00] <abentley> leonardr, even findTeam returns IPerson.
[16:00] <leonardr> abentley: let's say we had a magic way of knowing. if we detected a non-inheritance scenario, we'd throw an exception. that's exactly what'll happen now: an exception saying 'a bar-type object doesn't have a property "onlypresentinfoo"'
[16:01] <leonardr> the 'people' collection i refer to is the client-side collection in launchpadlib
[16:01] <leonardr> on the server side, we declare the most general interface (though findTeam really should return ITeam). on the client side, we expect the most specific interface
[16:02] <leonardr> that is, it's guaranteed that everything coming out of PersonSet implements at least IPerson
[16:03] <leonardr> but if you grab an object from PersonSet, and you know *nothing* about it, the most convenient thing to do for your end-user is to let them treat it as ITeam if they think it's an ITeam, and raise an exception if that assumption turns out to be invalid
[16:04] <leonardr> that decision could go the other way, and this branch would still be needed, but launchpadlib wouldn't be as useful
[16:05] <leonardr> you couldn't do launchpad.people['team'].membership_policy
[16:22] <abentley> leonardr, don't you need to load the object to evaluate launchpad.people['team'].membership_policy ?
[16:23] <leonardr> only if 'membership_policy' is something that could conceivably be a property of launchpad.people['team']
[16:23] <leonardr> if you do launchpad.people['team'].nosuchproperty it won't load the object, you'll just get an error
[16:24] <leonardr> the question is whether or not membership_policy (or anything held by teams but not persons) should look like nosuchproperty
[16:26] <abentley> leonardr, it seems like it would be a programming error to try to access launchpad.people['team'].nosuchproperty and therefore making avoid retrieving the object is not a great advantage.
[16:27] <leonardr> i agree it's not a great advantage, but that's how it works now. a request will only go over the wire if it conforms with what the client knows about the application based on the wadl
[16:28] <leonardr> by contrast, launchpad.people['person'].membership_policy will go over the wire and _then_ raise an exception
[16:28] <leonardr> the important thing is that both cases raise exceptions
[16:29] <abentley> Can you give me an example of a case where there wouldn't be an exception?
[16:30] <leonardr> launchpad.people['person'].name or launchpad.people['team'].membership_policy
[16:30] <leonardr> in the first case, the property is present in both team and person
[16:30] <leonardr> in the second case, the property is only present for team, but fortunately the object happens to be a team (according to the server)
[16:34] <abentley> leonardr, can you give me an example where the object is a person, but appears to be a team?  The first one I can think of is dir(person)
[16:35] <leonardr> abentley, i'm about to push a launchpadlib branch that will hopefully make it clearer
[16:36] <leonardr> but, to answer your question, launchpad.people['person'] will appear to be a team until the request is made. so, yes, dir(person) and .lp_attributes will show team attributes
[16:36] <leonardr> once the request is made, dir(person) will change to be more accurate
[16:37] <leonardr> abentley: https://code.edge.launchpad.net/~leonardr/launchpadlib/shim-users/+merge/28067
[16:55] <krkhan> gmb: thanks for the review. i'll post on the list tonight for suggestions and see how it goes
[16:55] <gmb> krkhan, Cool.
[17:02] <abentley> leonardr, why doesn't setting "httplib2.debuglevel = 0" make the test always pass?
[17:03] <leonardr> abentley: going to go out on a limb here and suppose that you have the wrong idea about what httplib2.debuglevel does?
[17:04] <abentley> leonardr, could be.
[17:04] <leonardr> all it does is show the requests as they're being made, so that i can demonstrate that requests are made later rather than sooner
[17:04] <leonardr> it doesn't affect the test environment itself
[17:07] <abentley> leonardr, I think that's what I expected.
[17:07] <abentley> leonardr, so why does line 56 show a GET?
[17:07] <leonardr> ah
[17:07] <leonardr> when you create a client object, it inherits the current value of httplib2.debuglevel
[17:08] <leonardr> it would be better to set httplib2.debuglevel later, for readability, but technically it doesn't matter--you can unset it anytime after you create the client
[17:08] <leonardr> i'll make the change
[17:08] <abentley> leonardr, okay, cool.
[17:09] <abentley> leonardr, it would be nice if you could create it with "debug_service = CookbookWebServiceClient(_http_debuglevel=1)" to reflect this.
[17:12] <leonardr> that's a good idea but i'd rather do that in a separate branch, since that would affect a bunch of tests
[17:13] <abentley> leonardr, that's fine.  I don't require it, just an idea.
[17:13] <leonardr> ok
[17:15] <abentley> Could you update the documentation to mention that WADL doesn't provide a way to find out inheritance relationships?
[17:16] <abentley> leonardr, also, did you consider allowing collections claim that they provide multiple types?
[17:17] <abentley> leonardr, that would allow a shim to be an IPersonOrITeam shim, and presumably it could find out which it is when it needs to.
[17:17] <leonardr> abentley: yes, that was my original (much more complicated plan), until flacoste said that team is and can only be a superset of person
[17:17] <leonardr> and the way this works in practice is very similar to that
[17:17] <leonardr> since team is a superset of person, IPersonOrTeam looks just like ITeam
[17:18] <leonardr> and any attempt to explicitly find out what kind of object a specific object is will collapse the wavefunction by making an http request
[17:19] <abentley> leonardr, I thought IPersonOrTeam would make http requests when dir(IPersonOrTeam) was invoked, etc.
[17:19] <leonardr> i see
[17:20] <james_w> salgado-lunch: https://code.edge.launchpad.net/~james-w/launchpad/copy-archive-test-refactor/+merge/28073 is the branch I was talking about if you could review please.
[17:20] <leonardr> i don't think that's worth the work, and if it is, we can do that work just as easily with this branch as a base
[17:24] <abentley> leonardr, approved, but I'd like to see an explanation of why you didn't consider inheritance (WADL doesn't provide that) in the docs.
[17:24] <leonardr> ok
[17:25] <leonardr> abentley, want to look at the launchpadlib branch too? (if not, gary has expressed an interest)
[17:25] <gary_poster> (a willingness :-) )
[17:26] <abentley> gary_poster, if you don't mind, there are some issues I'd like to chase up.
[17:26] <gary_poster> abentley, leonardr, will do
[17:41] <salgado> james_w, I'll get to it in a minute
[17:41] <james_w> thanks
[17:48] <gary_poster> leonardr: launchpadlib branch approved, no comments
[17:49] <leonardr> gary: cool. lazr.restfulclient release is out, launchpadlib release will follow, then i'm going to go to bed
[17:49] <gary_poster> ok, leonardr
[17:52] <leonardr> gary: done, see you tomorrow
[18:59] <sinzui> is anyone available for a 47 line fix: https://code.edge.launchpad.net/~sinzui/launchpad/unique-openid-identifiers-0/+merge/28081
[20:08] <sinzui> abentley, are you available for a 47 line fix: https://code.edge.launchpad.net/~sinzui/launchpad/unique-openid-identifiers-0/+merge/28081
[20:08] <abentley> sinzui, sure.
[20:13] <abentley> sinzui, openid identifiers are derived from account names?
[20:13] <sinzui> no random stringd
[20:13] <sinzui> sample data sucks and uses
[20:13] <abentley> sinzui, so the random strings aren't random enough?
[20:14] <sinzui> the randoms get reused apparently
[20:15] <sinzui> I saw something like merge-Z3dgy45  and Z3dgy45 in staging. the latter cannot be merged
[20:15] <abentley> sinzui, that doesn't sound like a good idea...
[20:15] <sinzui> We do not control openid identifiers
[20:15] <abentley> sinzui, Ah, that's canonical-foundations?
[20:16] <EdwinGrubbs> abentley: will you have time to review this branch? https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-35728-bugtracker-overlay/+merge/28100
[20:16] <abentley> EdwinGrubbs, I expect so.
[20:18] <abentley> sinzui, "dash which does not occur in SSO identifier." should probably be "dash, which does not occur in SSO identifiers."
[20:18] <sinzui> abentley,  ISD, canonical-identity-provider reuses identifier tokens. I was surprised. In my original fix I wrote that since we know the identifier is unique and cannot contain a dash, we could prepend 'merge-' and there was no chance of future conflict...
[20:19] <sinzui> I was proven wrong in 10 days
[20:20] <abentley> sinzui, if you are replying to my last statement, I was just nitpicking grammar.
[20:21] <sinzui> abentley, no, I was trying to provide backgroind to  "that's canonical-foundations?"
[20:21]  * sinzui is being pedantic, sorry
[20:22] <abentley> sinzui, ah, I missed that you mentioned ISD in there.
[20:23] <abentley> sinzui, I think it would be good to add a unit test to ensure that merging accounts works when you have duplicate identifiers.
[20:23] <sinzui> yes, I should do that
[20:23] <abentley> sinzui, the change itself looks fine.
[20:24] <abentley> sinzui, ping me when you have that test.
[20:24] <sinzui> okay
[20:24] <abentley> sinzui, also, this sinzui in the queue: is that the same thing?
[20:24] <sinzui> yep
[20:38] <abentley> EdwinGrubbs, could you explain 395-397, please?
[20:57] <EdwinGrubbs> abentley: it's not really important now that we normally run windmill tests through the test runner so we always have a clean database. I copied it from test_add_milestone. I can remove it if you want.
[20:58] <abentley> EdwinGrubbs, please do, then.
[22:05] <sinzui> abentley, check your email for my reply and incremental diff
[22:06] <abentley> sinzui, I am EOD.