这是indexloc提供的服务,不要输入任何密码
Skip to content

Allow updating Deployment delete_on_destroy field #27

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

Merged
merged 1 commit into from
May 5, 2022

Conversation

dglsparsons
Copy link
Collaborator

No description provided.

@dglsparsons dglsparsons requested a review from craigandrews May 4, 2022 07:43
if resp.Diagnostics.HasError() {
resp.Diagnostics.AddError(
"Error getting deployment plan",
"Error getting deployment plan",

Choose a reason for hiding this comment

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

nothing else worth adding here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I copy-pasted from the Create function. Might have a play and see what the output looks like 🤔. I suspect it might be one of those errors that's really hard to actually see, just because the schema validation kicks in before this point.

Choose a reason for hiding this comment

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

yeah it's worth figuring out if the errors raised here are actually useful for output, or if they're pretty much just flags to say "stop now, it didn't work"

diags = resp.State.Set(ctx, state)
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
return

Choose a reason for hiding this comment

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

two bare returns that don't surface the error

Copy link

@craigandrews craigandrews left a comment

Choose a reason for hiding this comment

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

Approved on the proviso that error outputs are investigated

@dglsparsons dglsparsons merged commit 318e2bf into main May 5, 2022
@dglsparsons dglsparsons deleted the allow-updating-delete-on-destroy branch May 5, 2022 08:45
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