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
Use nil
checks for defaults when applicable
#12099
Conversation
Changelog[uncommitted] (2023-02-09)Bug Fixes
|
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.
Outside of the generated code, do we have a test exercising the actual generated behavior we want? Specifically, the behavior you have in the PR description against a generated test type?
No, but there should be such a test. I'll add it. |
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.
Thanks for the test!
Made a couple minor tweaks but LGTM. Would recommend squashing the commits before merging since.
28befff
to
b13a570
Compare
bors r+ |
🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
12099: Use `nil` checks for defaults when applicable r=iwahbe a=iwahbe <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> Fixes #12106 # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> We can see some passed values not respected when applying default values to interface types when the value passed to the interface is a plan value. ```go type Args struct { Field pulumi.BoolPtrInput // default: true in the schema } (&Args{ Field: pulumi.BoolPtr(false) }).Defaults().Field // False (&Args{ Field: pulumi.Bool(false) }).Defaults().Field // True ``` See https://go.dev/play/p/aiuA7Jzy0mH for a complete example. We can fix this by switching the internal check of `Defaults()` to use a `== nil` check when an interface is being checked. ## Checklist <!--- Please provide details if the checkbox below is to be left unchecked. --> - [ ] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Service, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. --> Co-authored-by: Ian Wahbe <ian@wahbe.com>
Build failed: |
Might need to regenerate |
Regnerate tests Add behavior test
b13a570
to
e15d0c5
Compare
bors r+ |
🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
12099: Use `nil` checks for defaults when applicable r=iwahbe a=iwahbe <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> Fixes #12106 # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> We can see some passed values not respected when applying default values to interface types when the value passed to the interface is a plan value. ```go type Args struct { Field pulumi.BoolPtrInput // default: true in the schema } (&Args{ Field: pulumi.BoolPtr(false) }).Defaults().Field // False (&Args{ Field: pulumi.Bool(false) }).Defaults().Field // True ``` See https://go.dev/play/p/aiuA7Jzy0mH for a complete example. We can fix this by switching the internal check of `Defaults()` to use a `== nil` check when an interface is being checked. ## Checklist <!--- Please provide details if the checkbox below is to be left unchecked. --> - [ ] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Service, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. --> Co-authored-by: Ian Wahbe <ian@wahbe.com>
Build failed: |
bors retry |
Build succeeded:
|
Fixes #12106
Description
We can see some passed values not respected when applying default values to interface types when the value passed to the interface is a plan value.
See https://go.dev/play/p/aiuA7Jzy0mH for a complete example.
We can fix this by switching the internal check of
Defaults()
to use a== nil
check when an interface is being checked.Checklist
make changelog
and committed thechangelog/pending/<file>
documenting my change