Safeguard method signatures for backwards compatibility

[Duncan McGreggor]

In an IRC discussion with Jamu about some syntax usage in a branch, he expressed concern over our use of extended call syntax with txAWS methods. In particular, his concern is with a method like this:

    def method_name(*some_parameter):
        ...

where any future changes in the method's signature will break backwards compatibility: those who are calling with arguments (represented by some_paremeter above) will have broken code if we ever need to amend the method signature to something like this:

    def method_name(a, *some_parameter):
        ...

or this:

    def method_name(a, b, c="", d="", *some_parameter):
        ...

A simple solution for this will be to instead use a keyword parameter for a list of items where we currently use extended call:

    def method_name(some_parameter=None):
        ...

[Duncan McGreggor (oubiwann)]

For reference, here's the IRC chat that led to this ticket:

[13:31] <oubiwann> jkakar: quick note before I head off to the bank
[13:31] <oubiwann> jkakar: extended call syntax is the name for Python's use of *args, **kwds in signatures
[13:32] <oubiwann> jkakar: for example, therve used *snapshot_ids in his txAWS method sig
[13:32] <oubiwann> back in a bit!
[13:48] <jkakar> oubiwann: Oh, I see. So you want me to do that with names I guess.
[13:56] <oubiwann> jkakar: yeah, that would be cool... mostly because it's consistent with the other implementations, and also because the code is nicely succinct
[13:56] <jkakar> oubiwann: Done and pushed. :)
[13:57] <oubiwann> jkakar: sweet!
[13:57] <jkakar> oubiwann: I actually think it's a bad choice to design the API like that, though.
[13:57] <jkakar> oubiwann: It basically makes it impossible to extend the function signature to take anything other than a name.
[13:57] <oubiwann> jkakar: because it's not explicit?
[13:57] <jkakar> oubiwann: So, if DescribeSecurityGroups in a future version takes a "group name" parameter, it gets hard to add.
[13:58] <jkakar> oubiwann: No, because it means you can only add things as keyword arguments. There's no possibility to add a positional argument that isn't considered part of *names without breaking backwards compatibility.
[13:58] <jkakar> oubiwann: But, I suspect the reality is that it's six of one, half dozen of the other, so not really a big deal.
[13:59] <oubiwann> jkakar: you can do stuff like this with extended call syntax: def doit(a, b, c="", d="", *args, **kwds)
[13:59] <oubiwann> jkakar: or are you talking about something else?
[14:01] <jkakar> oubiwann: Yeah, I know you can, but any change to positional arguments like that will break backwards compatibility with methods that have a *args as the first parameter, like we do with EC2Client.describe_security_groups.
[14:09] <oubiwann> jkakar: ah, I see what you're saying
[14:09] <jkakar> oubiwann: Yeah, not a huge deal, but that kind of thing makes me wary of extended call syntax.
[14:10] <oubiwann> jkakar: yeah, I can see that
[14:10] <oubiwann> jkakar: with the AWS API, we're pretty much screwed anyway, though :-(
[14:11] <oubiwann> when we upgrade to support the next version, there's likely to be a lot of extra work
[14:12] <jkakar> oubiwann: Maybe, but I think the use of extended call syntax makes it more likely that we'll be screwed because we're creating a situation that makes changing the API in a backwards-compatible way harder than if it wasn't used.
[14:12] <oubiwann> jkakar: we may have to move to something similar to what the landscape client does with versioned APIs, and keeping a base API that is version-independent
[14:12] <oubiwann> jkakar: hrm
[14:12] <jkakar> oubiwann: Yeah, I could see that.
[14:12] <oubiwann> jkakar: you're tempting me to not use extended call ;-)
[14:13] <jkakar> oubiwann: Although, the base API in Landscape isn't exactly version-independent, we've just been lucky and careful so far. :)
[14:13] <oubiwann> hehe
[14:13] <jkakar> oubiwann: :)
[14:15] <oubiwann> jkakar: how about this for a compromise...
[14:16] <oubiwann> jkakar: def describe_snapshots(self, snapshot_...

[Jamu Kakar (jkakar)]

I think in actuality, the realistic cases where using extended call syntax would cause problems will even if you don't have them. Even so, as a stylistic thing, I don't like them because when I read code like foo("bar") it's not obvious I can pass in more than one "bar". With foo(bars=["bar"]) it's obvious.

Anyway, I think this mostly comes down to personal style.