[12:02] Do we still require the backquotes for """See IThisClass.""" docstrings? [12:13] It's still listed here: https://dev.launchpad.net/PythonStyleGuide#Docstrings [12:22] noodles775: Thanks! Then that settles it for me, and I'll continue to be a prick about it. :-) [12:28] 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. === matsubara-afk is now known as matsubara === mrevell is now known as mrevell-lunch === mrevell-lunch is now known as mrevell === abentley changed the topic of #launchpad-reviews to: on-call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [15:32] abentley, can i trouble you to review https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/server-takes-precedence/+merge/28059 ? [15:32] leonardr, sure. [15:33] leonardr, why are 'team' and 'person' different types? === matsubara is now known as matsubara-afk [15:35] abentley: 'team' has some features like membership_policy that 'person' doesn't have [15:35] same reason ITeam and IPerson are different interfaces [15:43] leonardr, it seems like this behaviour should require that the returned type is derived from the expected type, don't you think? [15:55] 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] but there's no way to represent inheritance in wadl, so we can't be much more sophisticated [15:57] 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] 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] leonardr, even findTeam returns IPerson. [16:00] 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] the 'people' collection i refer to is the client-side collection in launchpadlib [16:01] 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] that is, it's guaranteed that everything coming out of PersonSet implements at least IPerson [16:03] 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] that decision could go the other way, and this branch would still be needed, but launchpadlib wouldn't be as useful [16:05] you couldn't do launchpad.people['team'].membership_policy === salgado is now known as salgado-lunch [16:22] leonardr, don't you need to load the object to evaluate launchpad.people['team'].membership_policy ? [16:23] only if 'membership_policy' is something that could conceivably be a property of launchpad.people['team'] [16:23] if you do launchpad.people['team'].nosuchproperty it won't load the object, you'll just get an error [16:24] the question is whether or not membership_policy (or anything held by teams but not persons) should look like nosuchproperty [16:26] 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] 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] by contrast, launchpad.people['person'].membership_policy will go over the wire and _then_ raise an exception [16:28] the important thing is that both cases raise exceptions [16:29] Can you give me an example of a case where there wouldn't be an exception? [16:30] launchpad.people['person'].name or launchpad.people['team'].membership_policy [16:30] in the first case, the property is present in both team and person [16:30] 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] 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] abentley, i'm about to push a launchpadlib branch that will hopefully make it clearer [16:36] 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] once the request is made, dir(person) will change to be more accurate [16:37] abentley: https://code.edge.launchpad.net/~leonardr/launchpadlib/shim-users/+merge/28067 [16:55] gmb: thanks for the review. i'll post on the list tonight for suggestions and see how it goes [16:55] krkhan, Cool. [17:02] leonardr, why doesn't setting "httplib2.debuglevel = 0" make the test always pass? [17:03] abentley: going to go out on a limb here and suppose that you have the wrong idea about what httplib2.debuglevel does? [17:04] leonardr, could be. [17:04] 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] it doesn't affect the test environment itself [17:07] leonardr, I think that's what I expected. [17:07] leonardr, so why does line 56 show a GET? [17:07] ah [17:07] when you create a client object, it inherits the current value of httplib2.debuglevel [17:08] 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] i'll make the change [17:08] leonardr, okay, cool. [17:09] leonardr, it would be nice if you could create it with "debug_service = CookbookWebServiceClient(_http_debuglevel=1)" to reflect this. [17:12] 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] leonardr, that's fine. I don't require it, just an idea. [17:13] ok [17:15] Could you update the documentation to mention that WADL doesn't provide a way to find out inheritance relationships? [17:16] leonardr, also, did you consider allowing collections claim that they provide multiple types? [17:17] 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] 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] and the way this works in practice is very similar to that [17:17] since team is a superset of person, IPersonOrTeam looks just like ITeam [17:18] 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] leonardr, I thought IPersonOrTeam would make http requests when dir(IPersonOrTeam) was invoked, etc. [17:19] i see [17:20] 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] 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] 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] ok [17:25] abentley, want to look at the launchpadlib branch too? (if not, gary has expressed an interest) [17:25] (a willingness :-) ) [17:26] gary_poster, if you don't mind, there are some issues I'd like to chase up. [17:26] abentley, leonardr, will do === salgado-lunch is now known as salgado [17:41] james_w, I'll get to it in a minute [17:41] thanks === deryck is now known as deryck[lunch] [17:48] leonardr: launchpadlib branch approved, no comments [17:49] gary: cool. lazr.restfulclient release is out, launchpadlib release will follow, then i'm going to go to bed [17:49] ok, leonardr [17:52] gary: done, see you tomorrow === deryck[lunch] is now known as deryck === sinzui changed the topic of #launchpad-reviews to: on-call: abentley || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [18:59] is anyone available for a 47 line fix: https://code.edge.launchpad.net/~sinzui/launchpad/unique-openid-identifiers-0/+merge/28081 [20:08] abentley, are you available for a 47 line fix: https://code.edge.launchpad.net/~sinzui/launchpad/unique-openid-identifiers-0/+merge/28081 [20:08] sinzui, sure. === abentley changed the topic of #launchpad-reviews to: on-call: abentley || reviewing: sinzui || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [20:13] sinzui, openid identifiers are derived from account names? [20:13] no random stringd [20:13] sample data sucks and uses [20:13] sinzui, so the random strings aren't random enough? [20:14] the randoms get reused apparently [20:15] I saw something like merge-Z3dgy45 and Z3dgy45 in staging. the latter cannot be merged [20:15] sinzui, that doesn't sound like a good idea... [20:15] We do not control openid identifiers [20:15] sinzui, Ah, that's canonical-foundations? [20:16] 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] EdwinGrubbs, I expect so. === EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: abentley || reviewing: sinzui || queue: [sinzui, Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [20:18] sinzui, "dash which does not occur in SSO identifier." should probably be "dash, which does not occur in SSO identifiers." [20:18] 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] I was proven wrong in 10 days [20:20] sinzui, if you are replying to my last statement, I was just nitpicking grammar. [20:21] abentley, no, I was trying to provide backgroind to "that's canonical-foundations?" [20:21] * sinzui is being pedantic, sorry [20:22] sinzui, ah, I missed that you mentioned ISD in there. [20:23] 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] yes, I should do that [20:23] sinzui, the change itself looks fine. [20:24] sinzui, ping me when you have that test. [20:24] okay [20:24] sinzui, also, this sinzui in the queue: is that the same thing? [20:24] yep === abentley changed the topic of #launchpad-reviews to: abentley || reviewing: Edwin || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [20:38] EdwinGrubbs, could you explain 395-397, please? === abentley changed the topic of #launchpad-reviews to: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [20:57] 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] EdwinGrubbs, please do, then. === abentley changed the topic of #launchpad-reviews to: is On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [22:05] abentley, check your email for my reply and incremental diff [22:06] sinzui, I am EOD. === salgado is now known as salgado-afk