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