| StevenK | wgrant: https://code.launchpad.net/~stevenk/launchpad/bulk-eid-to-obj/+merge/171705 should address one of your concerns with export-pu-auditor | 02:39 |
|---|---|---|
| wgrant | StevenK: Looks pretty good, but needs a few fixes | 03:31 |
| wgrant | StevenK: Also, person merges | 03:32 |
| StevenK | wgrant: Person merges? | 03:38 |
| wgrant | StevenK: Person merges | 03:39 |
| wgrant | . | 03:40 |
| StevenK | wgrant: Repeating yourself does not help with more context/ | 03:43 |
| StevenK | s/\//./ | 03:43 |
| wgrant | StevenK: Person merges! | 03:46 |
| wgrant | How do they interact with your branches? | 03:46 |
| StevenK | wgrant: In terms of the eid branch, we can't do anything. If a person is converted to an eid and stored in auditor, and then merged into person, we will return the first person because we store by id | 03:47 |
| wgrant | Why can't you do anything? | 03:48 |
| wgrant | There's a well-defined successor to a merged person | 03:48 |
| wgrant | person.merged | 03:48 |
| StevenK | Ugh | 03:49 |
| StevenK | Why can't we have simple things | 03:49 |
| StevenK | This function already hurt my brain, and then I had to twist it to the side to understand your issues about its duplication, and now I have to deal with person merges. | 03:50 |
| wgrant | What duplication? | 03:50 |
| StevenK | "These are redundant. type_ids[foo] is just obj_id_to_eid[foo].keys()." | 03:51 |
| wgrant | Oh, that thing | 03:51 |
| wgrant | Do you understand now? | 03:51 |
| StevenK | Yes, I've fixed it. | 03:51 |
| wgrant | They are equivalent, except that type_ids[foo] has duplicates | 03:51 |
| wgrant | Right | 03:51 |
| StevenK | Tests even pass. | 03:51 |
| StevenK | wgrant: http://pastebin.ubuntu.com/5803604/ | 03:52 |
| wgrant | Ah | 03:53 |
| wgrant | Now that has a slight behaviour change | 03:53 |
| wgrant | Which you probably want a test for | 03:53 |
| wgrant | What happens when you ask for an ID that doesn't exist? | 03:53 |
| wgrant | In the current version on LP it will give you None | 03:53 |
| wgrant | In your new version it will omit the key | 03:53 |
| wgrant | In another version it may crash | 03:53 |
| StevenK | Yeah, that was probably why I had it at the top, but you complained. | 03:54 |
| wgrant | Also, scheme is annoying | 03:54 |
| wgrant | I'd say something like "instance, cls, id = eid.split(':')" | 03:54 |
| wgrant | I think that's what they are | 03:54 |
| wgrant | But I can't tell right now, because scheme is rather opaque :) | 03:55 |
| wgrant | StevenK: I think omitting them from the response is probably best, so your new behaviour is correct | 03:55 |
| wgrant | But we likely want a test for that | 03:55 |
| wgrant | So we don't accidentally start crashing if an object disappears | 03:55 |
| StevenK | But then auditor client will ask for them and then give KeyError | 03:56 |
| wgrant | Isn't this on the other side of auditorclient? | 03:56 |
| wgrant | We're parsing its responses | 03:57 |
| wgrant | Not giving it things to ask for | 03:57 |
| StevenK | Yes, we're parsing its responses | 03:57 |
| StevenK | So, we get back lp:PackageUpload:6 as the eid for obj | 03:58 |
| StevenK | Then the auditorclient calls enterpriseids_to_objects() and it omits lp:PackageUpload:6 because it doesn't exist. It then tries to map it to an object and crashes | 03:58 |
| wgrant | Yes | 04:00 |
| wgrant | Because enterpriseids_to_objects cannot know what behaviour the callsite wants | 04:00 |
| wgrant | It has to be the callsite's responsibility to handle a missing object | 04:00 |
| StevenK | Oh, I can use get anyway | 04:00 |
| wgrant | Of course | 04:01 |
| wgrant | That's probably the correct course of action for that particular case | 04:01 |
| StevenK | wgrant: http://pastebin.ubuntu.com/5803623/ | 04:04 |
| StevenK | In terms of person merges, I have NFI | 04:06 |
| wgrant | StevenK: existant isn't a word, and that assertEqual formatting is odd, but otherwise that looks good | 04:08 |
| wgrant | Person merges don't belong in this branch | 04:08 |
| === tasdomas_afk is now known as tasdomas | ||
| StevenK | wgrant: That MP is updated. | 04:19 |
| wgrant | StevenK: k | 04:23 |
| StevenK | wgrant: http://pastebin.ubuntu.com/5803648/ should resolve another of your export-pu-auditor issues | 04:24 |
| StevenK | Oh, I didn't drop scheme | 04:25 |
| wgrant | I'd also prefer those two maps to be renamed as I said in my first comment, but it's at least a bit less confusing now | 04:26 |
| StevenK | wgrant: scheme death, and maps renamed: http://pastebin.ubuntu.com/5803653/ | 04:28 |
| wgrant | Right | 04:30 |
| StevenK | wgrant: One more with feeling for that MP. | 04:39 |
| StevenK | GAH | 04:41 |
| StevenK | YUI 3.10.3 was released 2 days after 3.10.2 | 04:41 |
| wgrant | Any notable fixes? | 04:42 |
| wgrant | We don't care about tracking the x.x.1 increments unless they have relevant fixes | 04:42 |
| StevenK | wgrant: The .swf vuln fixed in 3.10.1 snuck back into .2 | 04:42 |
| StevenK | So they fixed it again for .3 | 04:43 |
| wgrant | Hah | 04:46 |
| StevenK | I don't think we serve any of the two swfs in our build tree | 04:47 |
| StevenK | But convoy will probably return them if asked | 04:47 |
| StevenK | 3.11 looks to have some nice speedups, too | 04:48 |
| * StevenK waits for YUI 3.11 JavaScript for Workgroups | 04:48 | |
| StevenK | wgrant: Do you approve of the auditorclient diff? | 05:35 |
| wgrant | StevenK: Sounds reasonable | 05:43 |
| ttx | wgrant: applied your requested fixes, except the use of ISpecificationTarget, see my comment at https://code.launchpad.net/~ttx/launchpad/lp1193389/+merge/171491 | 09:42 |
| === BradCrittenden is now known as bac | ||
| === BradCrittenden is now known as bac | ||
| === benji__ is now known as benji | ||
| === benji is now known as Guest32120 | ||
| === wedgwood_away is now known as wedgwood | ||
| === tasdomas is now known as tasdomas_afk | ||
| === matsubara is now known as matsubara-lunch | ||
| === matsubara-lunch is now known as matsubara | ||
| === wedgwood is now known as wedgwood_away | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!