-
Notifications
You must be signed in to change notification settings - Fork 3
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
Keep httpEndpoint
valid
#185
Conversation
Does the PR have any schema changes?Does the PR have any schema changes?Looking good! No breaking changes found. Maintainer note: consult the runbook for dealing with any breaking changes. |
Fixes broken test options required for new test case.
010711d
to
446fce3
Compare
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 is very similar to what caused us problems in with aws.rds.Instance
. Could we remove the patch all-together and apply a similar solution to pulumi/pulumi-aws#2686 (keeping all of our code in resources.go
).
This way there won't be any unintended interactions within the upstream provider.
This is an alternative strategy to restore an old field, and does not require a patch in the upstream provider. Instead, we create the field in resources.go and make sure that if a user sets it, we move it to `restEndpoint` (which the provider still has as valid).
httpEndpoint
valid
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'm approving to remove the "(requested changes)" from my review, but I would like a second pair of eyes before I merge.
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.
Looks good to me, thank you!
I have a personal preference to using the plain string as a lookup key rather than assigning to a const but it's definitely not a blocker 😄
We do this by re-projecting the field into the TF provider (without logic) and then ensuring that the upstream provider never sees the field. This fixes #181.
We also: