Registry task upgrade #743
Conversation
| @@ -21,10 +21,13 @@ class GetTaskResult: | |||
| The task that has the ARM resource and task properties. | |||
| The task will have all information to schedule a run against it. | |||
| """ | |||
| def __init__(__self__, agent_configuration=None, creation_date=None, credentials=None, id=None, identity=None, location=None, name=None, platform=None, provisioning_state=None, status=None, step=None, tags=None, timeout=None, trigger=None, type=None): | |||
| def __init__(__self__, agent_configuration=None, agent_pool_name=None, creation_date=None, credentials=None, id=None, identity=None, is_system_task=None, location=None, log_template=None, name=None, platform=None, provisioning_state=None, status=None, step=None, system_data=None, tags=None, timeout=None, trigger=None, type=None): | |||
There was a problem hiding this comment.
Not specifically related to this PR, but this signature change could break positional arg usage. I'm not sure if it's worth appending new parameters rather than alphabetizing to avoid this. (Sacrifices readability in favor of compatibility)
There was a problem hiding this comment.
Oh, interesting. Do you know how we handle this in TF-based providers?
There was a problem hiding this comment.
I'm not sure. My guess is that we're clobbering the existing signature in the same way. Most users are likely to use kwargs given the long list of options on most resources, but this could lead to subtle breaks if not.
There was a problem hiding this comment.
Yeah, it looks like we also reorder them somewhat arbitrarily: https://github.com/pulumi/pulumi-gcp/pull/558/files#diff-8bea7c3056bab4675291789291ba2a00711e860f76383317e117686401f61abb
There was a problem hiding this comment.
It doesn't look arbitrary to me. If something becomes Optional then it has a default value passed in, and so it has to go after the args that don't have default values.
At least, the gcp example doesn't look arbitrary to me. It looks like the example commented on here is sorted by alphabetical order.
There was a problem hiding this comment.
A couple things worth noting here:
GetTaskResultis the result of an invoke, which we instantiate internally. It's likely very rare that anyone would be calling its__init__.- For
TArgsclasses, I've deliberately added a*parameter, which forces users to use kwargs to avoid this issue with positional arguments.
There was a problem hiding this comment.
But yeah, I don't know how common it is for folks to use positional args. And in fact, with the new ResourceArgs changes in the python SDK I'm not sure it's even possible to use positional args now.
There was a problem hiding this comment.
It doesn't look arbitrary to me
Yes, probably my bad wording. I meant that any argument may get a default value in a minor release and end up changing the position.
But it looks like you have it under control 👍
Use the
2019-06-01-previewversion for thecontainerregistry.Taskresource, fix #736