| === daniel is now known as Guest8611 | ||
| === Guest8611 is now known as Odd_Bloke | ||
| smoser | rharper: chad and i are in hangout | 17:13 |
|---|---|---|
| rharper | right | 17:13 |
| * rharper comes up for air | 17:13 | |
| dojordan | @smoser, added the ephemeraldhcpv4 content manager: https://code.launchpad.net/~dojordan/cloud-init/+git/cloud-init/+merge/334341 | 18:29 |
| blackboxsw | dojordan: just got through a pass on it | 19:59 |
| smoser | good. i was just abou to ask blackboxsw to do that :) | 19:59 |
| blackboxsw | heh, was just about to ask you if we care about EphemeralDCHPv4 returning {} instead of None on failed lease find, so it'll always return a dict either filled with dhcp options on success, or empty on failure | 20:00 |
| blackboxsw | otherwise we get unhelpful Tracebacks if callsites try to lease['myopt | 20:00 |
| blackboxsw | otherwise we get unhelpful Tracebacks if callsites try to lease['myopt' | 20:00 |
| blackboxsw | otherwise we get unhelpful Tracebacks if callsites try to lease['myopt'] on a NoneType | 20:00 |
| blackboxsw | <- back to typo school with you | 20:01 |
| smoser | that was a good pointp you made there. | 20:01 |
| smoser | iguess its __enter__ could raise a exception | 20:02 |
| smoser | which coudl be caught | 20:02 |
| smoser | i like blackboxsw 's feedback. | 20:04 |
| blackboxsw | #achievementunlocked :) | 20:05 |
| dojordan | cool, i like it. so thoughts on empty dict vs exception? | 20:07 |
| dojordan | I'm leaning empty dict | 20:08 |
| dojordan | actually, the nice thing about the exception is its already in a try catch | 20:08 |
| smoser | i think checking for None. explicitly. while a lease with no keys would be wierd, checking for None is just more expclcit. | 20:08 |
| dojordan | explicit is better than implicit :) | 20:09 |
| blackboxsw | good points. | 20:10 |
| blackboxsw | ok so leaving None return value then, and callsites should explicity check for it? | 20:10 |
| dojordan | on the other hand, the whole point of the EphemeralDHCPv4 context manager is to do dhcp then call the Ephemeral nic with it. if that fails, the whole thing is busted | 20:11 |
| dojordan | so we shouldn't even still be in the context manager IMO | 20:12 |
| blackboxsw | +1 maybe we actually just raise the exception from there instead of return None.... it's what open's context manager does if file doesn't exist | 20:14 |
| dojordan | then inside DataSourceEc2 we can catch the exception and return None | 20:15 |
| blackboxsw | maybe a new NoDHCPLeaseError (newly defined in cloudinit.net.dhcp) | 20:15 |
| dojordan | also, re: the changing info to lease inside ec2, can we just avoid the variable since the code inside the ctxmgr doesn't reference it | 20:16 |
| dojordan | @blackboxsw, should we reuse the InvalidDHCPLeaseFileError? | 20:20 |
| dojordan | or catch it and throw a NoLeaseError? | 20:21 |
| smoser | yeah.. DataSourceEc2 is why i had it return None | 20:22 |
| smoser | exception is fine. | 20:22 |
| dojordan | @smoser, @blackboxsw, look again? | 21:11 |
| blackboxsw | dojordan: +1 two minor comments on the latest changes. I'll test on azure/ec2 but I' | 21:24 |
| blackboxsw | think I'm good there assuming you make the minor changes | 21:24 |
| blackboxsw | ... 'this round' of minor changes ;) | 21:25 |
| smoser | https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/336366 | 21:50 |
| smoser | powersj: ^ that actually worked this time in my local test. | 21:50 |
| smoser | first pass through i wasn't getting all the botos patched up correctly | 21:50 |
| powersj | looking | 21:52 |
| powersj | smoser: so if bytes doesn't exist, then we will print the whole 'output' of ec2 console to console? that may fill things up | 21:53 |
| smoser | powersj: well, it'll fill the test too. | 21:56 |
| smoser | thats only 64k at most | 21:56 |
| smoser | if you want i can make it pring only the first X chars | 21:56 |
| smoser | powersj: http://paste.ubuntu.com/26419112/ ? | 21:57 |
| powersj | smoser: sure - have you seen it fail and drop down to that? | 21:58 |
| smoser | well, only when i was not getting it patched correctly | 21:59 |
| smoser | the first version only patched the 'ec2_resource' | 21:59 |
| smoser | but then you had created instances using a session that was not using theh patched path | 21:59 |
| smoser | or something like that. | 21:59 |
| smoser | either way, yes. i saw it, but if we hit it it means that the "monkey patch" is probably broken | 21:59 |
| dojordan | @blackboxsw, fixed, good catch | 22:04 |
| dojordan | @blackboxsw, sorry to bother, but any idea when you're able to get around to the azure/ec2 pass? | 23:43 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!