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.

Sign up for GitHub

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

Jump to bottom

Improve CustomResource for Python SDK #700

Merged
merged 12 commits into from
Aug 23, 2019
Merged

Improve CustomResource for Python SDK #700

merged 12 commits into from
Aug 23, 2019

Conversation

lblackstone
Copy link
Member

@lblackstone lblackstone commented Aug 8, 2019 •
edited
Loading

  • Standardize arg names with the rest of the SDK
  • Add typing
  • Add .get method

Fixes #699

@lblackstone lblackstone force-pushed the lblackstone/python-customresource branch from 29b181a to a1b9146 Compare August 13, 2019 21:55
@lblackstone lblackstone marked this pull request as ready for review August 13, 2019 21:57
@lblackstone lblackstone force-pushed the lblackstone/python-customresource branch from 2c3733d to 966e9a5 Compare August 13, 2019 21:57
@lblackstone lblackstone force-pushed the lblackstone/python-customresource branch from ec5c7ff to c561dd1 Compare August 14, 2019 23:13
@lblackstone
Copy link
Member Author

lblackstone commented Aug 14, 2019 •
edited
Loading

@CyrusNajmabadi I realized that I wasn't unwrapping the Inputs properly, so preview would fail if the id, etc were Outputs. I updated the .get method accordingly.

Assuming that looks ok, I'll need to update all the other .get methods as well in a followup PR.

@CyrusNajmabadi
Copy link
Contributor

@CyrusNajmabadi I realized that I wasn't unwrapping the Inputs properly, so preview would fail if the id, etc were Outputs. I updated the .get method accordingly.

I'm not certain i understand that. On the TS side we've marked things as interface ResourceOptions { id?: Input<String> }. But on the python side we don't seem to allow outputs... something seems weird here. Looking more deeply.

@lblackstone lblackstone force-pushed the lblackstone/python-customresource branch from 3a78a25 to c7da4c1 Compare August 19, 2019 17:36
@lblackstone
Copy link
Member Author

@CyrusNajmabadi Any further feedback? If not, I'd like to get this merged today.

@lblackstone lblackstone force-pushed the lblackstone/python-customresource branch from c7da4c1 to 126c447 Compare August 20, 2019 18:19
@lukehoban lukehoban mentioned this pull request Aug 21, 2019
Closed
3 tasks
@lblackstone lblackstone force-pushed the lblackstone/python-customresource branch from da8cc18 to e3d769c Compare August 22, 2019 20:39
@lblackstone lblackstone force-pushed the lblackstone/python-customresource branch from 5854fd5 to c9f9fcb Compare August 23, 2019 00:23
@lblackstone lblackstone merged commit 5a5747e into master Aug 23, 2019
@pulumi-bot pulumi-bot deleted the lblackstone/python-customresource branch August 23, 2019 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukehoban lukehoban lukehoban left review comments

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix CustomResource in Python SDK
3 participants
@lblackstone @CyrusNajmabadi @lukehoban