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.

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

[DO NOT MERGE] Upstream v2.33.0 with 272df8b #39

Closed
wants to merge 3 commits into from

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented May 21, 2024

This PR exists to show the changes between upstream-v2.33.0-with-272df8b and upstream-v2.33.0. It should never be merged.

The first commit (28b1672) re-applies a previous fix that upstream providers rely on.

93be5f4 is a empty commit.

0c0bf0d fixes a merge issue, reverting the current state of the fork back to upstream. It strictly reduces the change set in our fork.

@iwahbe iwahbe self-assigned this May 21, 2024
iwahbe added a commit to pulumi/pulumi-terraform-bridge that referenced this pull request May 21, 2024
Stacked on top of #1998

---

This PR allows provider2.upgradeState to handle 64 bit integer numbers.
The substance of the change is
pulumi/terraform-plugin-sdk#39.

The only change here is a test.
@VenelinMartinov
Copy link

VenelinMartinov commented May 21, 2024

should we revert 8ce6686 too?
Seems like it should be part of the revert now.

@iwahbe
Copy link
Member Author

iwahbe commented May 21, 2024

should we revert 8ce6686 too? Seems like it should be part of the revert now.

I don't think we need to.

// UseJSONNumber should be set when state upgraders will expect
// json.Numbers instead of float64s for numbers. This is added as a
// toggle for backwards compatibility for type assertions, but should
// be used in all new resources to avoid bugs with sufficiently large
// user input. This field is only valid when the Resource is a managed
// resource.
//
// See github.com/hashicorp/terraform-plugin-sdk/issues/655 for more
// details.
UseJSONNumber bool

Reverting wouldn't improve our git history nor do we want to take a change there. This part matches upstream.

@VenelinMartinov
Copy link

// Fork: We only add this for backward compatibility purposes. This
// bool is not used in any of the codebase and serves only when a resource
// has opted into `UseJSONNumber` in the resource definition. This only
// is in place to allow providers to compile.

This is unlikely to be upstream and now incorrect, right?

@iwahbe iwahbe closed this Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants