| 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/ | 02:48 |
| === rinze changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| rinze | sinzui, hi | 10:36 |
| === matsubara-afk is now known as matsubara | ||
| 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:55 |
| salgado | noodles775, sure, I have it in my inbox and will get to it shortly | 12:56 |
| noodles775 | salgado: thanks. | 12:56 |
| salgado | noodles775, in which case is the sreg extension not included in the response? | 13:06 |
| 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:07 |
| noodles775 | It's not seen in the current code as _getEmailAddressAndFullName is only called if the account doesn't exist. | 13:08 |
| 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:10 | |
| salgado | indeed, duped | 13:11 |
| noodles775 | Yeah, I'd assumed the sreg response should always be there, and was surprised too. | 13:12 |
| 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:14 |
| 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:15 |
| salgado | noodles775, I think I've found the problem | 13:16 |
| 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 |
| === rinze is now known as jelmer-lunch | ||
| salgado | s/bug/but | 13:17 |
| 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:18 |
| salgado | I think it's fine to include the 3 of them, but the nickname shouldn't be used anywhere | 13:19 |
| noodles775 | OK, I'll update it... thanks salgado ! | 13:20 |
| 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:22 |
| salgado | noodles775, sure, I can do that | 13:24 |
| noodles775 | Ta. | 13:25 |
| 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:27 |
| 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:28 |
| 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:29 |
| noodles775 | Yes, but that is the long-term plan. | 13:30 |
| salgado | cool! | 13:31 |
| 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:38 |
| noodles775 | That won't be required... I'd only added it because of the above issue. | 13:39 |
| 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:40 |
| 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:48 |
| 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:50 | |
| noodles775 | OK, pushed r9515... | 13:52 |
| 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:02 |
| salgado | cool, let me reload again | 14:04 |
| 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:31 |
| salgado | noodles775, np | 14:39 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [sinzui, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| 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:57 |
| jtv | jelmer-lunch: my windmill tests pass now... I think there was some change in setup that we both needed to synchronize to. | 14:59 |
| === jelmer-lunch is now known as jelmer_ | ||
| jelmer_ | noodles775: ok | 15:00 |
| jelmer_ | jtv: I'll have another look, thanks. | 15:01 |
| === jelmer_ is now known as Guest58376 | ||
| === Guest58376 is now known as rinze | ||
| noodles775 | Sure. | 15:08 |
| === deryck is now known as deryck[lunch] | ||
| jtv | noodles775: anything to add to jelmer's review? | 16:24 |
| noodles775 | jtv: I pasted our irc conv. on the mp. | 16:28 |
| 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:30 |
| jtv | rinze: the tests on my JS branch are passing now, repeatedly and consistently... do I get your Approval vote? | 16:31 |
| jtv | noodles775: if you approve of the js, maybe you can hit the js review button? | 16:32 |
| 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:33 |
| 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:35 |
| 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:36 |
| 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:37 |
| === bigjools changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [sinzui, noodles775, bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === rinze changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: noodles775 || queue: [bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| * 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:43 |
| 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:44 |
| 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:45 |
| bigjools | both fixed | 16:46 |
| === henninge_ is now known as henninge | ||
| === salgado is now known as salgado-lunch | ||
| 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:54 |
| rinze | bigjools: Tests pass here and I don't see any other issues. | 16:55 |
| 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 | 16:56 |
| 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:02 |
| bigjools | ok pushed up again | 17:03 |
| 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:05 |
| === rinze changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| henninge | jtv: review sent (finally) | 17:18 |
| jtv | ahhh! | 17:18 |
| 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:19 |
| henninge | ;-) | 17:20 |
| === deryck[lunch] is now known as deryck | ||
| === rinze changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| bigjools | is anyone around for a simple but urgent review? | 18:05 |
| === salgado-lunch is now known as salgado | ||
| === matsubara is now known as matsubara-lunch | ||
| === matsubara-lunch is now known as matsubara | ||
| === rockstar changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| * 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:49 |
| bigjools | attaboy | 19:50 |
| bigjools | I still learn stuff 3.5 years after I started on it.... | 19:50 |
| rockstar | bigjools, why do you "create copy archives for rebuild testing"? That's not a PPA thing, right? | 19:53 |
| 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:54 |
| 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:55 |
| rockstar | Er, why shouldn't it look like the normal distro pages? | 19:56 |
| bigjools | they're overkill for this type of archive, mainly | 19:56 |
| 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:08 |
| james_w | it's a third type | 20:12 |
| rockstar | james_w, which is? | 20:14 |
| rockstar | Is it like a throwaway archive just for rebuilding copies of existing packages? | 20:14 |
| 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:16 |
| 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:17 |
| * 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:22 |
| bigjools | unfortunately yes | 20:24 |
| james_w | damn you bigjools, I've now started typing poopy instead of poppy | 20:25 |
| 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:27 |
| 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:28 |
| bigjools | james_w: haha | 20:29 |
| bigjools | rockstar: thanks for the review | 20:38 |
| 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:39 |
| 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 | 20:40 |
| === matsubara is now known as matsubara-afk | ||
| === salgado is now known as salgado-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!