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

Trigger recipient fixes #311

Merged
merged 13 commits into from
May 31, 2023
Merged

Trigger recipient fixes #311

merged 13 commits into from
May 31, 2023

Conversation

jharley
Copy link
Collaborator

@jharley jharley commented May 30, 2023

This completes the move of the honeycombio_trigger resource to the Terraform Plugin Framework (started in #306).

In order to support versions of Terraform core older than 1.4.0 (which, is rather "fresh") there is a bit of logic to handle Null or Unknown values of these awkwardly-shaped notification recipients.

This also 'bumps' the version of Terraform core used in CI to 1.0.11 (from the old 0.14.x series).


Copy link
Contributor

@brookesargent brookesargent left a comment

Choose a reason for hiding this comment

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

Nice work! 🚀

@@ -92,6 +92,7 @@ const (
PDSeverityERROR PagerDutySeverity = "error"
PDSeverityWARNING PagerDutySeverity = "warning"
PDSeverityINFO PagerDutySeverity = "info"
PDDefaultSeverity = PDSeverityCRITICAL
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice add for extra clarity

}

func (m notificationRecipientsModifier) PlanModifySet(ctx context.Context, req planmodifier.SetRequest, resp *planmodifier.SetResponse) {
// Do nothing on resource destroy.
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for the comments here to explain the logic going on!

Default: stringdefault.StaticString("critical"),
Optional: true,
Computed: true,
Description: "The severity to set with the PagerDuty notification. If no severity is provided, 'critical' is assumed.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice add!

@jharley jharley marked this pull request as ready for review May 30, 2023 19:40
@jharley jharley requested a review from a team as a code owner May 30, 2023 19:40
@jharley jharley merged commit 75760e6 into main May 31, 2023
@jharley jharley deleted the jharley.trigger-recipient-fixes branch May 31, 2023 14:01
LukeCarrier added a commit to LukeCarrier/terraform-provider-honeycombio that referenced this pull request Jul 5, 2023
It looks like these missing returns are an oversight from honeycombio#311?
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.

Changing recipient details for Slack type recipient in honeycombio_trigger causes 422 failure
2 participants