-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[codegen/python] - Implement dynamic config-getters #7447
Conversation
af138f5
to
465811f
Compare
Diff for pulumi-random with merge commit 01d5fa1 |
Diff for pulumi-azuread with merge commit 01d5fa1 |
Diff for pulumi-random with merge commit 1c8a51e |
Diff for pulumi-azuread with merge commit 1c8a51e |
Diff for pulumi-gcp with merge commit 01d5fa1 |
Diff for pulumi-gcp with merge commit 1c8a51e |
Diff for pulumi-azure with merge commit 1c8a51e |
Diff for pulumi-aws with merge commit 01d5fa1 |
Diff for pulumi-aws with merge commit 1c8a51e |
465811f
to
0bcd069
Compare
Diff for pulumi-random with merge commit 2797193 |
fa80cbb
to
9156c3a
Compare
Diff for pulumi-random with merge commit b58eea8 |
Diff for pulumi-random with merge commit a502b1e |
df1bd26
to
8dc80bb
Compare
Diff for pulumi-random with merge commit 7b1a923 |
Diff for pulumi-random with merge commit 63887d3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two initial comments after taking a quick look
...gen/internal/test/testdata/provider-config-schema/python/pulumi_configstation/config/vars.py
Outdated
Show resolved
Hide resolved
...gen/internal/test/testdata/provider-config-schema/python/pulumi_configstation/config/vars.py
Outdated
Show resolved
Hide resolved
import sys | ||
from .vars import _ExportableConfig | ||
|
||
sys.modules[__name__].__class__ = _ExportableConfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤯
I feel a little lost situating what this is for, do we have a ticket list? I'll try to read again next week with a fresh head. By the implementation of it, it sounds like schema can define config variables, types and defaults, and the PR makes these "type-safely" (I'd say statically) available to the autogenerated Python resource provider code. What's tripping me up is "dynamic", use of auto API, and what exactly are the bw-compat concerns. |
@t0yv0 this is for the python part of #5582 - sorry, I had that in the PR description but I guess it got dropped at some point. The problem is defined pretty well in the issue, let me know if you need any more information. The issue that @justinvp brought up about the types is somewhat orthogonal to the specific problem that this PR is solving but it came up because I tried to emit the output types and they are incorrect. |
Diff for pulumi-random with merge commit f107b64 |
6bfb0ec
to
7e1a638
Compare
Diff for pulumi-random with merge commit 622c097 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise.
It might be worth adding a comment that explains briefly why we're generating the _ExportableConfig
and using sys.modules[__name__].__class__ = _ExportableConfig
, and why we're generating the stub file.
7e1a638
to
db6dee5
Compare
Description
This PR implements dynamic provider config getters for python. To avoid a breaking change, we define a class that extends the ModuleType type and implements property getters for each config key. We then overwrite the
__class__
attribute of the current module as described in the proposal for PEP-549.To continue to get autocomplete for the config keys, we now emit a
__init__.pyi
stub file in the config module which declares the variables and associated types.I have added tests for the codegen side of things, but I'm struggling to figure out how to test the functionality in isolation. I used the following Automation API script to locally validate the result, but it requires a locally built pulumi_aws based on the codegen in this PR.
Python part of #5582
Notable Changes
vars.py
config/__init__.py
config/__init__.pyi
Checklist