Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sdk/python] Handle unknown results from methods #7677

Merged
merged 7 commits into from
Jul 29, 2021
Merged

Conversation

justinvp
Copy link
Member

This fixes handling of unknown results from methods in the Python SDK. Node.js and Go were already handling this appropriately.

This adds tests in each of the languages. The test program passes an unknown value to the provider, and asserts that an apply is not run for it, and then returns the value, and the program asserts that an apply does not run on the result. The Python test program fails before the fix and succeeds after.

Fixes #7670

@justinvp justinvp requested review from t0yv0 and pgavlin July 29, 2021 14:26

resolve_value.set_result(value)
resolve_is_known.set_result(True)
resolve_is_known.set_result(is_known)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel we're coding against a very low-level interface here so it sounds like it was an easy mistake to make.

@t0yv0
Copy link
Member

t0yv0 commented Jul 29, 2021

Curious, the integration tests are fine here but how do you feel about using Mock-based tests instead? I think this could run against a MockMonitor and exercise the changes in rpc.py but maybe I'm mistaken. They're a bit more tedious but run faster. Or would that lose essential coverage?

Update the existing Python methods test to include a type check in the method result class, to match what would be generated from the SDK, for some extra coverage.
@justinvp
Copy link
Member Author

how do you feel about using Mock-based tests

Good point. We could probably have Mock-based tests for this, but I think I'd still additionally want integration tests to test E2E. Also, at some point soon I hope to be able to add SDK generation for many of these integration tests (generate SDKs for the provider resources based on a schema, to fully exercise the codegen + runtime), so in some ways this is setting the stage for that.

@justinvp justinvp merged commit 74ab9e0 into master Jul 29, 2021
@pulumi-bot pulumi-bot deleted the justin/call_unknown branch July 29, 2021 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[sdk/python] Methods are not correctly bubbling up unknown values
3 participants