dojordan@smoser, I was able to test on artful successfully as well. I also addressed all the CR comments: https://code.launchpad.net/~dojordan/cloud-init/+git/cloud-init/+merge/33434100:18
dojordan@smoser, another day another iteration pushed. Can you please take a look when you get a chance: https://code.launchpad.net/~dojordan/cloud-init/+git/cloud-init/+merge/33434119:25
smoserblackboxsw: http://paste.ubuntu.com/26412425/19:25
smoserwhat do you think of that19:25
smoseri'm basically out of things ic an complain about in dojordan's branch :)19:26
smoserso that paste, would simplify some things.19:26
smoserinterested in dojordan's feedback too19:26
smoserand i'd like for blackboxsw to take a look at the branch there to see if i'ive missed anyting19:26
smoser(i really don't like 'unknown-245', ... that really should be handled centrally *somewhere* , but thats not dojordan's fault that it is how it is)19:27
dojordanI like it, only comment is there are some differences between ec2 and azure that make a generic context manager not perfect19:37
dojordani.e. calculating the bcast addr, and the stupid unknown-245 dhcp option...19:38
smoserwell, we can just h andle that if they're there.19:47
smoserer... in that the general one can use them.19:48
smoserif the dhcp server responded with those options, then... you probably should use them :)19:48
dojordantrue, i guess calculating the bcast addr is pretty generic. want me to incorporate your patch then?19:50
smoseryeah. if you're ok with that.19:51
smoseri really appreciate your willingness to take feedback19:51
dojordansounds good, and sure, it looks much cleaner19:52
blackboxswhrm sorry wrong channel20:30
blackboxswI like the cool factor of nested context managers, but I wonder if we really even need  EphemeralIPv4Network at all smoser. The only consumer was ec2, and azure would now be looking to use the new EphemeralDHCPv4 as well.20:37
blackboxswcouldn't we just repurpose EphemeralIPv4Network to do all this for us?20:38
blackboxswI don't really have a strong concern, though existing pastebin doesn't properly accept the fallback_nic param and the context manager return value should be named lease instead of info for clarity20:42
smoserblackboxsw: well i think that probably digitalocean *should* use EphemeralIPv4Network20:43
blackboxswI do like encapsulating that maybe_dhcp handling inside the context manager20:43
blackboxswgood point.20:44
blackboxswand nevermind on the accepting fallback_nic param. I saw that now in __init__ my bad20:45
smoserblackboxsw: i didn't know if i should do the dhcp in the __init__ or in the __enter__20:46
smoseri could come up with arguments for both.20:46
blackboxswyeah, I *think* init makes sense here20:46
blackboxswand I think on line 41 you meant +        if not self.ephipv4:20:47
dojordan(stupid question) what benefit do you get from doing work in init vs enter?20:55
dojordani guess if you instantiated a context manager w/out a with statement?20:55
blackboxswI'm onboard with the introduction of the EphemeralDHCP context manager. that logic for interacting with EphemerlIPv4Network is kinda klunky. it's best to obfuscate that anyway from DatasourceEc2.20:58
blackboxswI actually think pylint prefers no __enter__ additional params as it looks like it raises an unexpected special method signature. I just had my wires crossed and thought we had generally set those params up in __enter__ instead of __init__21:00
blackboxswI can't seem to find  examples supporting extra params in __enter__21:03
dojordanso with CtxMgr(a): would call __init__ with a?21:07
blackboxswcorrect a would be passed in as param 1 to init21:08
dojordanso theoretically, it's not possible to pass additional parms to __enter__?21:08
blackboxswand I can't  create a context manager  class where I'm able to do that.... but I'm just toying around as we speak.21:09
smoserdojordan: init versus __enter__21:12
smoserits really just a usage thing21:12
blackboxswahh so __init__ is performed at the same time as __enter__ when we use with statement21:12
smoseryou could then do:21:12
blackboxswwe could init separately and call with later proving the args I think21:12
smoser x = EphemeralDHCPv4("eth0")21:12
smoser with x:21:12
smoser   ...21:12
smoser  with x:21:12
smoser   ...21:12
smoser(bad indentation there)21:12
smoserif you did the dhcp in __init__ then you wouldnt *re* dhcp each 'with' (__enter__)21:13
smoserif you only use the class in one 'with' then they're equivalent.21:13
dojordanbut you can't pass parameters to __enter__ but you can to init?21:14
smoserblackboxsw: when you do:21:14
smoseri think that is correct :)21:14
smoserdojordan: ^21:14
smoserwith myclass(name=value) as fp:21:15
smoserbasically does:21:15
smoser <myclass>.init(name=value)21:15
smoser then fp = <result>.__enter__()21:15
smoserat least thats how i understand it21:15
dojordancool :)21:16
dojordannext question, is it possible to call __exit__() and __enter__ again?21:16
dojordani.e. when we raise an exception, we go to the next loop iteration and create a new instance of the context manager. is there a better way?21:17
smoserwell i fyou have the class around you can.21:20
smoserbut as i did an that paste there isnt a lot of value in keeping the class around for re-use, right?21:20
smoserwhere as if it reused the old lease there would be value, but thats not what you want21:20
dojordanright, i see21:21
dojordanmakes sense21:21

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!