| thumper | lifeless: sure | 00:55 |
|---|---|---|
| lifeless | thanks | 01:14 |
| thumper | lifeless: done | 01:34 |
| lifeless | thanks | 01:38 |
| jtv | thumper: while you're at it, can you review this one for me? https://code.launchpad.net/~jtv/launchpad/recife-policy-mixin/+merge/40300 | 02:47 |
| jtv | (that was deliberately phrased for a flippant rebuttal, so feel free if it's not convenient) | 02:48 |
| thumper | jtv: if it was small I'd say yes, but I'm in the middle of something. Sorry. | 02:48 |
| jtv | no worries | 02:49 |
| * jtv scans for victims | 02:49 | |
| jtv | bac! | 02:49 |
| jtv | Are you up for a non-small review? | 02:49 |
| bac | jtv i can be. can it wait a little bit as i just started something? | 02:49 |
| jtv | bac: yes it can | 02:50 |
| lifeless | ahhh the sweet smell of real urls in the morning | 02:51 |
| lifeless | it smells like victory | 02:51 |
| * jtv finds this obvious crack in lifeless' sanity a good reason to ask: | 02:58 | |
| jtv | you're suggesting a standalone class to implement translation policy. Were you thinking of a schema change? | 02:59 |
| lifeless | jtv: I wasn't, but perhaps one is needed. | 02:59 |
| lifeless | s/needed/possibly beneficial. | 03:00 |
| jtv | I see it making a lot of sense at this point. | 03:00 |
| lifeless | we have -way- to many mixins | 03:00 |
| lifeless | when you've got 1 mixin its a problem. 20+ mixings is arggggh zomg mode | 03:00 |
| jtv | Yes, we do have too many of them. | 03:01 |
| jtv | And in this case, there's actual state that's being replicated into multiple classes—basically a denormalized object design. | 03:02 |
| lifeless | is there a good reason for that ? | 03:02 |
| jtv | Best I can come up with is "progressing insight." | 03:05 |
| jtv | In other words, "no but that may not have been so obvious at the time." | 03:05 |
| jtv | There's 2 identical fields in Product, ProjectGroup, and Distribution. | 03:05 |
| jtv | I'm trying to expose and unify the common patterns. | 03:06 |
| jtv | Extracting it into a separate database table could be a lot of work, of course. | 03:08 |
| jtv | Some joins will change shape. | 03:08 |
| jtv | The new objects' lifetimes have to be managed etc. | 03:09 |
| jtv | Too much work for now. I think I'll go with a standalone non-db class first. | 03:10 |
| jtv | lifeless: I'm looking at implementing this as an adapter. | 03:24 |
| jtv | lifeless: so you'd say "ITranslationPolicy(pillar).makeSomeDecision()" | 03:25 |
| jtv | but you can also adapt certain other objects to get the ITranslationPolicy that applies to them. | 03:25 |
| jtv | Does that make sense to you? | 03:26 |
| lifeless | that seems a decent start | 03:28 |
| lifeless | as for adapter vs pillar.translation_policy - shrug, its largely just spelling (except those pesky lifetimes that you mentioned) | 03:29 |
| jtv | Yes, and I don't just want to keep adding to the interfaces. | 03:36 |
| jtv | bac, lifeless: reworking the policy object so that it's not a mixin becomes too much work for this branch; I'll have leave that for a separate branch. | 03:50 |
| * jtv goes out for a bite | 03:55 | |
| === jtv is now known as jtv-eat | ||
| === jtv-eat is now known as jtv | ||
| stub | lifeless: Should https://code.launchpad.net/~stub/launchpad/update-storm/+merge/40264 go through without review? It consists entirely of unicode casts, updates to modern Storm syntax and delints at 2.4k lines of diff. | 08:26 |
| bac | jtv: return the favor? | 09:27 |
| jtv | bac: honour compels me. | 09:28 |
| bac | jtv: lucky for you i'm not as prolific. a measly 88 line diff. https://code.launchpad.net/~bac/launchpad/bug-671539/+merge/40309 | 09:28 |
| jtv | bac: prolific? I have more changes waiting that I carefully cut out of the diff to keep its size down. :-) | 09:28 |
| jtv | bac: I find the "if not person.is_team: enabled = True else: enabled = False" a bit hard to read. Have you considered this alternative? | 09:31 |
| jtv | owner_is_individual = (not person.is_team) | 09:31 |
| jtv | …(…, enabled=owner_is_invidual) | 09:31 |
| bac | jtv: yeah, i'll make that change. | 09:32 |
| jtv | Thanks. | 09:32 |
| bac | brain fart | 09:32 |
| bac | any other comments, jtv? | 10:21 |
| jtv | bac: whoops, got sidetracked! Getting back to it now. | 10:21 |
| jtv | bac: Shouldn't the section heading in the story test be "---"-underlined instead of "==="-underlined? | 10:22 |
| jtv | (I do not like rest-style headings) | 10:22 |
| jtv | bac: approved. | 10:27 |
| bac | jtv: i'll look at the formatting. i thought i had followed the style used in the file but perhaps i messed up | 10:36 |
| jtv | bac: or the previous author of the file got it wrong, or we just have a harmless inconsistency. | 10:36 |
| === allenaptoo is now known as allenap | ||
| === danilo_ is now known as danilos | ||
| === matsubara-afk is now known as matsubara | ||
| === Ursinha-afk is now known as Ursinha | ||
| jtv | Hey, who's up for a review that I actually tried to make fun? https://code.launchpad.net/~jtv/launchpad/independence-for-ihastranslationtemplates/+merge/40321 | 12:50 |
| jtv | matsubara maybe? | 12:51 |
| jtv | gmb then? you're always in for a laugh, no? | 12:53 |
| matsubara | jtv, I'm not an official reviewer | 12:54 |
| jtv | ah! | 12:54 |
| jtv | well good morning, anyway. :) | 12:54 |
| === jtv changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| matsubara | good morning :-) | 12:57 |
| jtv | Still holding that T-shirt. :) | 13:00 |
| jtv | henninge, do you want to review it then? | 13:03 |
| henninge | jtv: oh, yes I should ... | 13:04 |
| jtv | Since it's your day and all :) | 13:04 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: - || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | Thanks. | 13:04 |
| henninge | Yes, I will have to find a replacement for next week. | 13:04 |
| jtv | We're not usually that picky about missing a review day. | 13:04 |
| henninge | I don't think I should be doing normal reviews *and* r-c reviews... | 13:04 |
| jtv | At least not on this side of the world, where usually there aren't any OCRs anyway. :) | 13:05 |
| jtv | henninge: why do you say normal reviews and r-c reviews? has there been a change that I'm unaware of? | 13:05 |
| henninge | jtv: I am release manager so I will be the one doing r-c reviews next Monday. | 13:06 |
| jtv | ah | 13:06 |
| jtv | I forgot that you were rc | 13:06 |
| henninge | rm | 13:06 |
| jtv | sorry, rm | 13:06 |
| jtv | rm -rf | 13:06 |
| henninge | In my world: Returned Missionary ;-) | 13:07 |
| jtv | (Seriously, I once saw Siemens advertise their OS called, if I remember the spelling correctly, RM/UNIX) | 13:07 |
| henninge | :-D | 13:07 |
| henninge | Those Germans... | 13:13 |
| === mrevell is now known as mrevell-lunch | ||
| === adeuring1 is now known as adeuring | ||
| allenap | henninge: Fancy taking on another branch? | 14:11 |
| === allenap changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: - || queue: [jtv, allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | henninge: I've put myself in the queue, but feel free to eject me. | 14:15 |
| henninge | jtv, allenap: Sorry, lunch got in the way. I will relocate now and look at your branches after that. | 14:19 |
| === mrevell-lunch is now known as mrevell | ||
| === salgado is now known as salgado-lunch | ||
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: jtv || queue: [allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-lunch | ||
| henninge | jtv: r=me Thank you. | 15:51 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: allenap || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | henninge: Thanks! | 16:03 |
| henninge | allenap: I have not yet used ARRAY's | 16:03 |
| henninge | s/'// | 16:03 |
| allenap | henninge: Dang, I misread and thought that r=me was for me :) Thank you anyway! | 16:06 |
| henninge | allenap: oh, I am sorry. | 16:06 |
| jtv | thanks henninge | 16:06 |
| allenap | henninge: Hehe, don't be sorry, it was my fault. | 16:07 |
| === matsubara-lunch is now known as matsubara | ||
| === deryck is now known as deryck[lunch] | ||
| abentley | henninge: is [jtv] already reviewed? | 16:34 |
| jtv | abentley: yes I am | 16:34 |
| === salgado-lunch is now known as salgado | ||
| === abentley changed the topic of #launchpad-reviews to: On call: henninge, abentley || Reviewing: allenap, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| henninge | ups, chansubjupd error | 16:34 |
| henninge | ups=oops(de_DE) | 16:35 |
| abentley | henninge: We should have a web page we can use to set the channel topic :-) | 16:35 |
| jelmer | abentley: Hi | 17:04 |
| jelmer | abentley: Can I add a MP to your queue? | 17:04 |
| abentley | jelmer: sure. | 17:04 |
| jelmer | abentley: https://code.launchpad.net/~jelmer/launchpad/672476/+merge/40349 | 17:05 |
| === deryck[lunch] is now known as deryck | ||
| allenap | henninge: I have to go soon, so ask soon if you have any questions about my branch :) | 17:31 |
| henninge | allenap: sorry, I was about to write the review. I actually understand what's going on now ... ;) | 17:32 |
| henninge | allenap: I don't have questions, though. You can look at it in the morning. Nothing serious just some suggestions. | 17:32 |
| allenap | henninge: Awesome, thank you. | 17:35 |
| === benji is now known as benji-lunch | ||
| === leonardr is now known as leonardr-lunch | ||
| === leonardr-lunch is now known as leonardr | ||
| abentley | jelmer: 59: s/iterabel/iterable | 18:20 |
| abentley | jelmer: Personally, I find using "with" with files more elegant than try/finally. | 18:21 |
| abentley | jelmer: Why do you test whether htpassword exists (and is a file) and delete it, instead of just overwriting it? | 18:23 |
| abentley | jelmer: is it secure to use token.person.name[:2] as a salt? | 18:25 |
| jelmer | abentley: Thanks, fixed. | 18:25 |
| jelmer | abentley: I usually don't bother with "with" out of fear of missing a "from __future__ import with_statment" somewhere | 18:26 |
| abentley | jelmer: we're all-2.6 now. You shouldn't import with_statement anymore. | 18:26 |
| jelmer | abentley: Overwriting it would work too I guess. That, and the token.person.name[:2] are simply the same as they were in the generate_ppa_htaccess.py script. | 18:27 |
| abentley | jelmer: yeah, I realize this is mostly just reorganizing old code. | 18:28 |
| abentley | jelmer: But using token.person.name[:2] as a salt is security-by-obscurity, and now that we're open-source, it's no longer obscure. | 18:29 |
| jelmer | abentley: it'd break a few existing tests though, so I wonder if I should leave that for a separate branch and file a high-priority bug about it for now. | 18:32 |
| jelmer | abentley: What do you think? | 18:33 |
| james_w | salts can be known | 18:33 |
| abentley | james_w: Oh? I would have thought that would make it trivial to forge the password. | 18:34 |
| james_w | abentley, no. Assuming you are talking about a real salt, and not something wrongly called that, they are not keys | 18:34 |
| james_w | if they were then they wouldn't be separate | 18:34 |
| abentley | james_w: we're talking about salts as used by the crypt C function. | 18:35 |
| james_w | what they do is prevent exploiting all passwords at once. If there were no salts you could generate one rainbow table and then decrypt everyone's ciphertext at once | 18:35 |
| james_w | adding a salt means that you have to attack each ciphertext in turn | 18:36 |
| james_w | so they do no need to be secret, but they do benefit from being well-distributed | 18:36 |
| abentley | james_w: okay, thanks for that clarification. | 18:36 |
| * jelmer didn't know that either | 18:37 | |
| james_w | (if num salts << num ciphertexts then you can decrypt a lot of ciphertexts with on rainbow table) | 18:37 |
| james_w | maybe a comment in the source? | 18:37 |
| abentley | james_w: and happy belated birthday. | 18:37 |
| james_w | thanks :-) | 18:37 |
| james_w | http://en.wikipedia.org/wiki/Salt_(cryptography) | 18:38 |
| james_w | it does say "For best security, the salt value is kept secret.", but if your system relies on the salt being secret, then the keys are probably too short anyway | 18:39 |
| abentley | james_w: it seems like in our context, all the passwords are equivalent, so only one cyphertext is needed. Therefore using distinct salts doesn't help? | 18:41 |
| james_w | abentley, where can I look to see the use of this in the source? | 18:42 |
| james_w | as I'm not sure what you mean by "all the passwords are equivalent" | 18:42 |
| abentley | james_w: https://code.launchpad.net/~jelmer/launchpad/672476/+merge/40349 | 18:42 |
| james_w | EACCESS | 18:43 |
| abentley | james_w: by "all the passwords are equivalent", I mean they all grant equivalent access AIUI | 18:43 |
| james_w | ah | 18:44 |
| james_w | so impersonation isn't really a problem | 18:44 |
| james_w | except for users who aren't supposed to have access at all | 18:44 |
| abentley | james_w: jelmer is refactoring this code: lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py | 18:44 |
| james_w | abentley, yeah, I'd agree, in this case salts aren't particularly useful for their normal use like in unix passwd | 18:46 |
| james_w | it adds a little benefit in that you wouldn't have to revoke every access if one token was compromised, but you may want to do that anyway to be sure | 18:47 |
| james_w | in this case though the main requirement is that crypt requires a salt, and so I think that the value used is as good as any other | 18:48 |
| abentley | james_w: Right. Hard to imagine only one token being compromised in this scenario. | 18:49 |
| abentley | james_w: okay. | 18:49 |
| === benji-lunch is now known as benji | ||
| abentley | jelmer: should _setupHtaccess consider the case where htaccess is different from what it would write? | 19:13 |
| jelmer | abentley: No, it sufficient that there is *some* htaccess there. | 19:13 |
| jelmer | abentley: generate_ppa_htaccess should take care of updating it | 19:14 |
| abentley | jelmer: ISTM that until generate_ppa_htaccess runs, the contents could be exposed. | 19:15 |
| abentley | jelmer: because the .htaccess might not impose any content restrictions. | 19:16 |
| jelmer | abentley: We only write out one kind of htaccess file at the moment | 19:16 |
| jelmer | abentley: I guess it might make sense take into account that we might want something different in that file in the future. | 19:17 |
| abentley | jelmer: so _setupHtaccess assumes that htaccess will not be different from what it would write. | 19:18 |
| jelmer | abentley: Sorry, yes. It assumes the .htaccess file would be the same and that there is some .htpasswd | 19:21 |
| abentley | jelmer: If an earlier process had died causing a partial write, that would not be true, but I think that's rare enough not to care about. | 19:23 |
| abentley | jelmer: I guess I'm wondering whether skipping writing htaccess and htpasswd is worthwhile. | 19:26 |
| jelmer | abentley: That's a good question, I've wondered about that as well. | 19:27 |
| abentley | jelmer: it seems like it doesn't save an appreciable amount of IO, and it does introduce a corner case. | 19:27 |
| jelmer | abentley: It saves us a single database query and a certain amount of IO. | 19:27 |
| jelmer | abentley: I don't know what the maximum number of subscriptions for a PPA is. | 19:28 |
| abentley | jelmer: At 243, I believe temp_filename is supposed to be on a new line. | 19:29 |
| abentley | jelmer: similarly at 351. | 19:31 |
| jelmer | abentley: Fixed | 19:34 |
| abentley | jelmer: Your tests would be a bit simpler if you returned the contents of .htaccess and .htpasswd as strings or if your functions took a file parameter. | 19:35 |
| abentley | jelmer: You could also just use useTempDir in your tests. | 19:36 |
| === lifeless_ is now known as lifeless | ||
| abentley | jelmer: You can use dedent to write your multi-line strings more neatly at 451. | 19:39 |
| jelmer | abentley: I haven't used dedent before, what is it? | 19:43 |
| jelmer | abentley: is this textwrap.dedent? | 19:44 |
| abentley | jelmer: Yes. It strips leading indents. | 19:44 |
| abentley | jelmer: Anyhow, those are just some ideas. r=me. | 19:48 |
| jelmer | abentley: Thanks, that was quite educational. | 19:50 |
| abentley | jelmer: np. | 19:51 |
| === matsubara is now known as matsubara-afk | ||
| jelmer | abentley: I just realized that if we always write out the htaccess file we need to honor the blacklist that currently exists in generate-ppa-htaccess.py | 20:12 |
| abentley | jelmer: I see. It's your call. | 20:13 |
| === salgado is now known as salgado-afk | ||
| === Ursinha is now known as Ursinha-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!