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. :)
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 > > >
