[sdk/{go,nodejs,python}] Fix DeletedWith resource option
#11883
Conversation
Changelog[uncommitted] (2023-01-16)Bug Fixes
|
| @@ -0,0 +1,24 @@ | |||
| // Copyright 2016-2023, Pulumi Corporation. All rights reserved. | |||
There was a problem hiding this comment.
It might be worth considering moving testprovider to its own repo and publishing it. Then we could use it from this repo and others (like pulumi-dotnet, pulumi-java, etc.), and we wouldn't need to have multiple copies of these hand-written files for calling the resources in the provider in each test that uses them.
There was a problem hiding this comment.
I think pretty much the entire integration test set should be callable from each language repo, not sure if that means testprovider needs to be moved out to it's own repo as well or not though. And as you'll have seen in dotnet I re-wrote the testprovider to C# in that repo, might make sense to do that for other languages as well.
justinvp
force-pushed
the
justin/deletedwith
branch
from
01674ff
to
35d1f86
Compare
This change fixes the `DeletedWith` resource option in the Go, Node.js, and Python SDKs and adds tests. This feature was a community contribution and while there were engine tests included with the original PR, there weren't any tests confirming the functionality worked correctly from each SDK. Here's a summary of the fixes: * Go: The `DeletedWith` resource option was never usable as it accepted a URN instead of a Resource. We discussed this internally a while back and decided to go ahead and fix this. (Note: While changing the signature is technically a breaking change, the feature is currently unusable, so the change would not break anyone, so there's no need to wait for a major version bump.) * Node.js: The `deletedWith` resource option did not work at all from the Node.js SDK because it was incorrectly passing the resource object itself in the RegisterResource request, rather than the resource's URN. * Python: The `deleted_with` resource option did not work at all from the Python SDK because it was incorrectly passing the resource object itself in the RegisterResource request, rather than the resource's URN. A `FailsOnDelete` resource has been added to the testprovider, which will fail when its `Delete` gRPC is called. The tests use this to ensure `Delete` is not called for resources of this type with the `DeletedWith` option specified.
justinvp
force-pushed
the
justin/deletedwith
branch
from
35d1f86
to
0b13f91
Compare
justinvp
changed the title
DeletedWith resource option
| @@ -0,0 +1,9 @@ | |||
| // Copyright 2016-2023, Pulumi Corporation. All rights reserved. | |||
There was a problem hiding this comment.
It is very lame we have to write each of these integration test 6 times (python, nodejs, go, dotnet, java, yaml). Are we ever going to prioritize matrix testing to fix this?
There was a problem hiding this comment.
Yes, we should talk more about how/when we can get to it.
|
bors merge |
|
Build succeeded: |
|
Thanks for taking this to the finish line |
This change fixes the
DeletedWithresource option in the Go, Node.js, and Python SDKs and adds tests.This feature was a community contribution and while there were engine tests included with the original PR, there weren't any tests confirming the functionality worked correctly from each SDK.
Here's a summary of the fixes:
Go: The
DeletedWithresource option was never usable as it accepted a URN instead of a Resource. We discussed this internally a while back and decided to go ahead and fix this. (Note: While changing the signature is technically a breaking change, the feature is currently unusable, so the change would not break anyone, so there's no need to wait for a major version bump.)Node.js: The
deletedWithresource option did not work at all from the Node.js SDK because it was incorrectly passing the resource object itself in the RegisterResource request, rather than the resource's URN.Python: The
deleted_withresource option did not work at all from the Python SDK because it was incorrectly passing the resource object itself in the RegisterResource request, rather than the resource's URN.A
FailsOnDeleteresource has been added to the testprovider, which will fail when itsDeletegRPC is called. The tests use this to ensureDeleteis not called for resources of this type with theDeletedWithoption specified.Fixes #11358
Fixes #11622