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

ENG-3380 Support currentVersion in p0_routing_rules #46

Merged
merged 2 commits into from
Dec 20, 2024

Conversation

fabgo
Copy link
Contributor

@fabgo fabgo commented Dec 19, 2024

Fix the p0_routing_rules resource so that it supports the currentVersion field required by the workflow POST API.

Verified the provider by creating and updating a number of basic routing rules.

Fix the p0_routing_rules resource so that it supports the currentVersion
field required by the workflow POST API.

Verified provider by creating and updating a number of basic routing rules.
@fabgo fabgo requested review from nbrahms and GGonryun December 19, 2024 22:31
Copy link
Contributor

@GGonryun GGonryun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! one small nit do we need all the debug logs? 🤔

@fabgo
Copy link
Contributor Author

fabgo commented Dec 20, 2024

lgtm! one small nit do we need all the debug logs? 🤔

I think they are useful to have for troubleshooting. They don't do anything unless debug logging is enabled.

if !current.Version.IsUnknown() && !current.Version.IsNull() {
currentVersion := current.Version.ValueString()
tflog.Debug(ctx, fmt.Sprintf("Current version: %s", currentVersion))
currentVersionPtr = &currentVersion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this mean that if someone changes the version manually on the UI, the terraform state will be invalid? What happens if you delete your current Tf state and start from scratch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the state is changed in the UI, it will work. Terraform does a read as part of the plan. It will show the differences and ask to apply them.

What this does is prevent Terraform from making changes if the state is concurrently modified after the plan is created.

If the TF state is deleted, it needs to be imported first. But that is existing behavior, not specific to this PR.

@fabgo fabgo merged commit 1eaaf10 into main Dec 20, 2024
14 checks passed
@fabgo fabgo deleted the fabian/eng-3380-fix-routing-rules branch December 20, 2024 18:11
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