[02:48] <lifeless> sinzui: if you're back, I've moved the profile module to lp.services as per your request, but it looks to me like it does not meet the namespace policy about deps. I've mailed the bug
[02:48] <lifeless> s/bug/mp/
[10:36] <rinze> sinzui, hi
[12:55] <noodles775> Hi salgado, if you've got a spare few mins, can you take a look at the issue I've identified at: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-get-or-create-from-identity/+merge/28885
[12:55] <noodles775> Let me know if I'm not making sense on the MP.
[12:56] <salgado> noodles775, sure, I have it in my inbox and will get to it shortly
[12:56] <noodles775> salgado: thanks.
[13:06] <salgado> noodles775, in which case is the sreg extension not included in the response?
[13:07] <noodles775> salgado: normal login with existing account. ie. 1) start dev server, 2) click login 3) enter admin@canonical.com:test 4) click login. In this case calling _getEmailAddressAndFullName raises the assert.
[13:08] <noodles775> It's not seen in the current code as _getEmailAddressAndFullName is only called if the account doesn't exist.
[13:10] <salgado> noodles775, that's weird because the code that generates the positive openid response is including the sreg extension unconditionally
[13:10]  * salgado tries to dupe
[13:11] <salgado> indeed, duped
[13:12] <noodles775> Yeah, I'd assumed the sreg response should always be there, and was surprised too.
[13:14] <salgado> noodles775, but this is a bug in the testopenid implementation
[13:14] <noodles775> Where should I look?
[13:14] <salgado> testopenid == the OpenID provider we use when developing/testing
[13:14] <salgado> I'm checking it now
[13:14] <salgado> noodles775, lib/lp/testopenid/browser/server.py
[13:15] <noodles775> OK, so we can be confident that the staging/production SSO always include the sreg.
[13:15] <salgado> createPositiveResponse() adds the sreg extension, but it's empty
[13:15] <noodles775> Thanks.
[13:16] <salgado> noodles775, I think I've found the problem
[13:17] <salgado> noodles775, OpenIDLogin.render() asks for 'email' and 'fullname' in the sreg response
[13:17] <salgado> bug testopenid ignores that and only includes the nickname
[13:17] <salgado> s/bug/but
[13:18] <noodles775> Right... I see.
[13:18] <noodles775> So should it instead include *only* the requested items (email/fullname) or in addition to the nickname?
[13:19] <salgado> I think it's fine to include the 3 of them, but the nickname shouldn't be used anywhere
[13:20] <noodles775> OK, I'll update it... thanks salgado !
[13:22] <noodles775> salgado: also, I assume you don't have time to review the actual branch, but if you could have a brief look and make sure you think it's a sane change, that would be great.
[13:24] <salgado> noodles775, sure, I can do that
[13:25] <noodles775> Ta.
[13:27] <salgado> noodles775, out of curiosity, why somebody needs a launchpad account to buy stuff from the software center?  wouldn't a Ubuntu SSO account be more appropriate in this case?
[13:28] <noodles775> salgado: yes, the must already have an SSO account to make the payment, it's after they've paid for the software, we need to give them access to a private PPA (ie. a private ppa subscription), and that's only possible if you've got an LP account.
[13:29] <salgado> I see
[13:29] <salgado> and it'd be lots of work to make it possible for someone to have a private ppa subscription without having a Launchpad profile, I assume?
[13:30] <noodles775> Yes, but that is the long-term plan.
[13:31] <salgado> cool!
[13:38] <salgado> 415	+        except LookupError:
[13:38] <salgado> 416	+            assert email_address is not None and full_name is not None, (
[13:38] <salgado> 417	+                "An email address and full name are required to "
[13:38] <salgado> 418	+                "create an account.")
[13:38] <salgado> noodles775, I'd move that assert out of the except block
[13:39] <noodles775> That won't be required... I'd only added it because of the above issue.
[13:40] <noodles775> Hrm, although, it's still possible for other callsites in the future to pass in None (even if processPositive.. doesn't).
[13:40] <noodles775> So yep, will do.
[13:40] <salgado> indeed
[13:48] <salgado> noodles775, I think you can get rid of checked_for_email and the two email_set.getByEmail(email_address) calls by doing "email = email_set.getByEmail(email_address)" at the beginning instead of "email = None"
[13:50] <noodles775> salgado: yep - that was part of r9514 which I've reverted after our discussion (I couldn't do it at the beginning at that point, because email was None when called via processPositive..)
[13:50] <salgado> right
[13:50] <salgado> I guess I should reload the m-p page, then
[13:50]  * noodles775 hasn't pushed... hangon.
[13:52] <noodles775> OK, pushed r9515...
[14:02] <salgado> noodles775, IPersonSet.getOrCreateByOpenIDIdentifier() has a requestor argument but the actual implementation doesn't
[14:02] <noodles775> salgado: thanks, I'll remove it from the IFace. I've just pushed 9516 which just adds the email/fullname to the sreg response for the testopenid server too.
[14:04] <salgado> cool, let me reload again
[14:31] <noodles775> Thanks salgado... I'll paste the irc conversation on the MP too. (And I've pushed the changes you suggested so far). I think that should be enough to get a normal review.
[14:39] <salgado> noodles775, np
[14:57] <noodles775> jelmer-lunch: if you've time when you get back: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-get-or-create-from-identity/+merge/28885
[14:59] <jtv> jelmer-lunch: my windmill tests pass now...  I think there was some change in setup that we both needed to synchronize to.
[15:00] <jelmer_> noodles775: ok
[15:01] <jelmer_> jtv: I'll have another look, thanks.
[15:08] <noodles775> Sure.
[16:24] <jtv> noodles775: anything to add to jelmer's review?
[16:28] <noodles775> jtv: I pasted our irc conv. on the mp.
[16:30] <jtv> noodles775: that's all about us being rusty and making dirty jokes afaics, nothing about the branch itself...
[16:30] <jtv> Ah, apart from the bit about henning's xxx that triggered the jokes
[16:31] <jtv> rinze: the tests on my JS branch are passing now, repeatedly and consistently... do I get your Approval vote?
[16:32] <jtv> noodles775: if you approve of the js, maybe you can hit the js review button?
[16:33] <noodles775> jtv: no, I was just making sure rinze hadn't missed anything obvious... I didn't do a proper review. As you mentioned in your MP, the branch is just cleaning up already working JS (outside of the chromium fix - just a different class)..
[16:33] <noodles775> Oh, but I can hit the js review button if you need it (sorry).
[16:33] <jtv> I'd love that, thanks!
[16:35] <rinze> jtv: Yeahp!
[16:35] <rinze> jtv: Updating the MP now
[16:35] <jtv> rinze: great!  Then I can still get it in before the deadline.
[16:36] <rinze> Also, if somebody here can review a oneliner: https://code.edge.launchpad.net/~jelmer/launchpad/fix-devel/+merge/29011
[16:36] <rinze> It's the testfix for devel
[16:37] <noodles775> rinze: done (and jtv too)
[16:37] <jtv> thanks
[16:37] <jtv> rinze: least I can do...
[16:37] <jtv> oh, too late :-)
[16:37] <rinze> jtv: noodles beat you to it, but thanks anyway :-)
[16:43]  * rinze looks at bigjools branch briefly
[16:43] <rinze> bigjools: there's a typo in paramter in your angry comment :-)
[16:43] <bigjools> I was too angry to speel
[16:44] <rinze> in lib/lp/soyuz/scripts/tests/test_populatearchive.py nonvirtualized seems to be a boolean parameter, yet it defaults to None and checks for "is not None"
[16:45] <rinze> I.e. if testCopyArchiveVirtualization specified nonvirtualized=False it would still not use virtualization
[16:45] <bigjools> rinze: yes deliberately, but I suppose it should default to False
[16:45] <bigjools> I was still too angry to think :)
[16:46] <bigjools> both fixed
[16:54] <rinze> bigjools: did you push your changes?
[16:54] <bigjools> rinze: no, I was waiting for more comments about my angry coding :)
[16:54] <rinze> hehe
[16:55] <rinze> bigjools: Tests pass here and I don't see any other issues.
[16:56] <rinze> perhaps you could add an assert somewhere that self.makeCopyArchive() can also still create archives that require virtualized builds
[16:56] <bigjools> yeah I saw that problem as you mentioned it too
[16:56] <bigjools> merde
[17:02] <rinze> bigjools: in your latest tip, the nonvirtualized argument to makeCopyArchive defaults to None rather than False
[17:02] <bigjools> rinze: some changes pushed up again
[17:02] <bigjools> ARGH
[17:02] <bigjools> missed it
[17:03] <bigjools> ok pushed up again
[17:05] <rinze> bigjools: I don't see the diff for that last change yet, but I trust that's correct :-) r=me
[17:05] <bigjools> thanks rinze :)
[17:05] <bigjools> saving me from my own stupidity on that branch
[17:18] <henninge> jtv: review sent (finally)
[17:18] <jtv> ahhh!
[17:19] <jtv> henninge: hope it's an approval!
[17:19] <henninge> jtv: almost
[17:19] <jtv> grrr
[17:19] <henninge> jtv: I mean, there are no big issues but I'd like to hear your opinions on some of my comments.
[17:20] <henninge> ;-)
[18:05] <bigjools> is anyone around for a simple but urgent review?
[19:49]  * bigjools meets rockstar here
[19:49] <rockstar> bigjools, I've come to the conclusion that soyuz is generally hard to understand unless you know the whole entire subsystem.
[19:49] <rockstar> The latter has become a new goal of mine.
[19:50] <bigjools> attaboy
[19:50] <bigjools> I still learn stuff 3.5 years after I started on it....
[19:53] <rockstar> bigjools, why do you "create copy archives for rebuild testing"? That's not a PPA thing, right?
[19:54] <bigjools> rockstar: no, it's more like a main archive presented like a PPA.  It allows us to copy source from somewhere else and rebuild it
[19:55] <rockstar> bigjools, presented like a PPA?  What's the use case there?
[19:55] <bigjools> UI code re-use :)
[19:55] <rockstar> (Patch looks fine, just trying to squeeze a few user stories out)
[19:55] <bigjools> we didn't want it looking like the normal distro pages
[19:55] <bigjools> the main use case is to find failed builds
[19:55] <rockstar> Why shouldn't it look the same?
[19:56] <rockstar> Er, why shouldn't it look like the normal distro pages?
[19:56] <bigjools> they're overkill for this type of archive, mainly
[20:08] <rockstar> bigjools, so I guess the thing I'm not understanding is what type of archive this is.
[20:08] <rockstar> Is it a distro archive or a PPA or something else I don't know about.
[20:12] <james_w> it's a third type
[20:14] <rockstar> james_w, which is?
[20:14] <rockstar> Is it like a throwaway archive just for rebuilding copies of existing packages?
[20:16] <james_w> rockstar: pretty much
[20:16] <rockstar> james_w, ah, okay.  That parses better now.
[20:16] <james_w> it's grown some extra features, but the primary use case is to copy sources from Ubuntu in to a new archive, and then rebuild them there
[20:17] <james_w> so the bug in question is where it copies a bunch of sources, and then fails while creating the build records for them
[20:17] <rockstar> Yeah, that makes sense.
[20:17] <james_w> in particular because the new archive doesn't have i386 enabled, and i386 is required to build arch "all" packages, it's the nominatedarchindep
[20:22]  * bigjools watches james_w explain how my branch works :)
[20:22] <bigjools> sorry - had to dash off, child-related poo emergency
[20:22] <james_w> nice
[20:22] <bigjools> actually it wasn't :(
[20:22] <rockstar> Yeah, I very much understand nominatedarchindep.  Remember that time that recipe builds were using the arm builder...
[20:24] <bigjools> unfortunately yes
[20:25] <james_w> damn you bigjools, I've now started typing poopy instead of poppy
[20:27] <james_w> rockstar: a small branch of my own if you would: https://code.launchpad.net/~james-w/launchpad/move-file-is-a-directory/+merge/29033
[20:28] <rockstar> james_w, I live serve...
[20:28] <rockstar> Er, I live to serve
[20:28] <james_w> \o/
[20:28] <rockstar> I apparently don't live to type correctly.
[20:29] <bigjools> james_w: haha
[20:38] <bigjools> rockstar: thanks for the review
[20:39] <rockstar> james_w, your branch is made of awesomeness.  Thanks for doing that.
[20:39] <bigjools> rockstar: aw jeez you Quoted me!
[20:39] <rockstar> One day, we'll have all our dependencies nice and neat.
[20:39] <rockstar> bigjools, yeah, it had to be done.
[20:40] <rockstar> bigjools, also, you're like, the 6th person today to tell me about poo related mishaps that occurred today.
[20:40] <bigjools> one day you will have kids and will enthusiastically join in
[20:40] <james_w> thanks rockstar