On Nov 26, 2014, at 12:48 PM, Eric Johnson <[email protected]> wrote:
> Thanks Lee! Yes, after sending this out, I'd pretty much settled on #1. I > figured a PR was the best way to have a more active discussion anyway. :) > Right, the dev list is usually somewhat quiet, but it does not mean it's not monitored. Definitely not option #3 , I was leaning for #2 until I read Lee's answer. Potentially a fourth way is to create a new method all together and deprecate the old one in a new major release. @Tomaz, what do you think ? -sebastien > > On Wed Nov 26 2014 at 9:21:08 AM Lee Verberne <[email protected]> wrote: > >> This has come up for me as well in adding support for targetHttpProxies. I >> like option 1 as well. Option 2 would mean adding a third None-defaulted >> parameter for targethttpproxy. Option 3 seems crufty. >> On Wed Nov 19 2014 at 6:39:48 AM Eric Johnson <[email protected]> wrote: >> >>> Hi folks, >>> >>> I'm working on the GCE driver and since this was first added to libcloud, >>> Google has made a change to the load-balancing 'forwarding rule'. When >>> first introduced the only option for traffic forwarding was to a >>> 'targetpool'. However, there is now an option to send traffic to a >>> 'targetinstance' as well. >>> >>> The current signature[1] expects a non-default argument of 'targetpool'. >>> Ideally, this argument would've been called 'target' to match the current >>> form of the GCE API and could take either a 'targetpool' or >>> 'targetinstance'. >>> >>> Could I get some input on the best way to proceed? >>> >>> Option 1) replace the ex_create_forwarding_rule()'s 'targetpool' >> positional >>> argument position with 'target' and move 'targetpool' to a None-default >>> argument. I think this would allow backwards compatibility if the new >>> 'target' is smart enough to check for either a GCETargetPool or >>> GCETargetInstance object. Anyone with code using the named argument of >>> 'targetpool' would continue to work fine. >>> >>> Option 2) Keep 'targetpool' as a value-required positional argument but >>> allow None. Also introduce a None-defaulted 'targetinstance' argument. >>> Users wanting to specify a GCETargetInstance, would then need to pass in >>> None for the 'targetpool' positional argument and use the new >>> 'targetinstance' argument. >>> >>> Option 3) Allow 'targetpool' to take either a GCETargetPool or >>> GCETargetInstance object and just document that the argument name should >>> really be called 'target'. >>> >>> I'm leaning toward #1, but am not confident that it's the best approach. >>> Any input or other ideas? >>> >>> [1] >>> https://github.com/apache/libcloud/blob/trunk/libcloud/ >>> compute/drivers/gce.py#L1102 >>> >>> Thank you, >>> Eric >>> >>
