txAWS

Bug #424543
Comment #1

Comment 1 for bug 424543

Revision history for this message

Duncan McGreggor (oubiwann) wrote on 2009-09-04:

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_ids=None)
[14:16] <oubiwann> this will let us keep the current usage inside the function, while safeguarding against backwards compatibl issues
[14:16] <jkakar> oubiwann: Yep, that's how I would do it.
[14:16] <oubiwann> jkakar: sweet
[14:16] <oubiwann> jkakar: okay, you've convinced me ;-)
[14:16] <jkakar> oubiwann: In fact, that's how I did describe_security_groups initially. :)
[14:16] <jkakar> oubiwann: Sweet!
[14:17] <jkakar> oubiwann: We should probably change all these things at once.
[14:20] <oubiwann> jkakar: so we're going to want to make sure that Thomas' Landscape code doesn't break with this change
[14:20] <oubiwann> jkakar: we might want to create a ticket for that...
[14:20] <jkakar> oubiwann: Oh wait, I don't want to change the extended call syntax thing in this branch.
[14:21] <jkakar> oubiwann: I think we should do it in a follow on branch. I don't think landing what I have will break anything, since it's all new code.
[14:21] <oubiwann> jkakar: +1
[14:22] <oubiwann> jkakar: I'll open a ticket now

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_ids=None)
[14:16] <oubiwann> this will let us keep the current usage inside the function, while safeguarding against backwards compatibl issues
[14:16] <jkakar> oubiwann: Yep, that's how I would do it.
[14:16] <oubiwann> jkakar: sweet
[14:16] <oubiwann> jkakar: okay, you've convinced me ;-)
[14:16] <jkakar> oubiwann: In fact, that's how I did describe_security_groups initially. :)
[14:16] <jkakar> oubiwann: Sweet!
[14:17] <jkakar> oubiwann: We should probably change all these things at once.
[14:20] <oubiwann> jkakar: so we're going to want to make sure that Thomas' Landscape code doesn't break with this change
[14:20] <oubiwann> jkakar: we might want to create a ticket for that...
[14:20] <jkakar> oubiwann: Oh wait, I don't want to change the extended call syntax thing in this branch.
[14:21] <jkakar> oubiwann: I think we should do it in a follow on branch.  I don't think landing what I have will break anything, since it's all new code.
[14:21] <oubiwann> jkakar: +1
[14:22] <oubiwann> jkakar: I'll open a ticket now