-
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
Set range in python codegen default SDK version #16151
base: master
Are you sure you want to change the base?
Set range in python codegen default SDK version #16151
Conversation
If the schema doesn't set any specific dependencies, we should be defaulting to requiring pulumi within the current major version rather than completely unconstrained. Discussion: https://github.com/pulumi/pulumi-kafka/pull/410/files#r1594324390
Changelog[uncommitted] (2024-05-09) |
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.
Reasonable. Probably need to regenerate snapshots with PULUMI_ACCEPT
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.
The comment here gives me pause, but even looking at the history it doesn't make a lot of sense here. (This seems to have been introduced in 8b98abd) I think my preference would be to just remove this shortcircuiting completely, since it seems unnecessary, and it can more easily fall out of sync with the code below.
Curious if other folks more familiar with codegen have opinions on this though.
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.
This seems reasonable as it matches what we do in other SDKs. Not sure there's any benefit to ever emitting the dependency without a version constraint.
As @Frassle points out, this will need to regenerate snapshots with PULUMI_ACCEPT
.
- Extract default SDK range to a const. - Update comments to reflect reality.
- We want to depend on the curret major version for python. This is being fixed in codegen here: pulumi/pulumi#16151 - The default dotnet range resolves to minimum version rather than the latest version which could be problematic.
tests in this folder need running with PULUMI_ACCEPT as well because they also generate SDKs. |
Currently unable to run these locally ... running:
Not sure if there's a pre-requisite step I've missed here? I ran Fails with the same error when running |
Description
If the schema doesn't set any specific dependencies, we should be defaulting to requiring pulumi within the current major version rather than completely unconstrained.
Discussion: https://github.com/pulumi/pulumi-kafka/pull/410/files#r1594324390
Checklist
make tidy
to update any new dependenciesmake lint
to verify my code passes the lint checkgofumpt
make changelog
and committed thechangelog/pending/<file>
documenting my change