=== daniel is now known as Guest8611
=== Guest8611 is now known as Odd_Bloke
smoserrharper: chad and i are in hangout17: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/33434118:29
blackboxswdojordan: just got through a pass on it19:59
smosergood. i was just abou to ask blackboxsw  to do that :)19:59
blackboxswheh, 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 failure20:00
blackboxswotherwise we get unhelpful Tracebacks if callsites try to lease['myopt20:00
blackboxswotherwise we get unhelpful Tracebacks if callsites try to lease['myopt'20:00
blackboxswotherwise we get unhelpful Tracebacks if callsites try to lease['myopt'] on  a NoneType20:00
blackboxsw<- back to typo school with you20:01
smoserthat was a good pointp you made there.20:01
smoseriguess its __enter__ could raise a exception20:02
smoserwhich coudl be caught20:02
smoseri like blackboxsw 's feedback.20:04
blackboxsw#achievementunlocked :)20:05
dojordancool, i like it. so thoughts on empty dict vs exception?20:07
dojordanI'm leaning empty dict20:08
dojordanactually, the nice thing about the exception is its already in a try catch20:08
smoseri think checking for None. explicitly.   while a lease with no keys would be wierd, checking for None is just more expclcit.20:08
dojordanexplicit is better than implicit :)20:09
blackboxswgood points.20:10
blackboxswok so leaving None return value then, and callsites should explicity check for it?20:10
dojordanon 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 busted20:11
dojordanso we shouldn't even still be in the context manager IMO20: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 exist20:14
dojordanthen inside DataSourceEc2 we can catch the exception and return None20:15
blackboxswmaybe a new NoDHCPLeaseError (newly defined in cloudinit.net.dhcp)20:15
dojordanalso, re: the changing info to lease inside ec2, can we just avoid the variable since the code inside the ctxmgr doesn't reference it20:16
dojordan@blackboxsw, should we reuse the InvalidDHCPLeaseFileError?20:20
dojordanor catch it and throw a NoLeaseError?20:21
smoseryeah.. DataSourceEc2 is why i had it return None20:22
smoserexception is fine.20:22
dojordan@smoser, @blackboxsw, look again?21:11
blackboxswdojordan: +1 two minor comments on the latest changes. I'll test on azure/ec2 but I'21:24
blackboxswthink I'm good there assuming you make the minor changes21:24
blackboxsw... 'this round' of minor changes ;)21:25
smoserpowersj: ^ that actually worked this time in my local test.21:50
smoserfirst pass through i wasn't getting all the botos patched up correctly21:50
powersjsmoser: so if bytes doesn't exist, then we will print the whole 'output' of ec2 console to console? that may fill things up21:53
smoserpowersj: well, it'll fill the test too.21:56
smoserthats only 64k at most21:56
smoserif you want i can  make it pring only the first X chars21:56
smoserpowersj: http://paste.ubuntu.com/26419112/ ?21:57
powersjsmoser: sure - have you seen it fail and drop down to that?21:58
smoserwell, only when i was not getting it patched correctly21:59
smoserthe first version only patched the 'ec2_resource'21:59
smoserbut then you had created  instances using a session that was not using theh patched path21:59
smoseror something like that.21:59
smosereither way, yes. i saw it, but if we hit it it means that the "monkey patch" is probably broken21:59
dojordan@blackboxsw, fixed, good catch22: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!