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

resource_pagerduty_service: skip response_play with "null" value #573

Merged
merged 2 commits into from
Oct 5, 2022

Conversation

Padorax
Copy link
Contributor

@Padorax Padorax commented Sep 22, 2022

Similar to fields "auto_resolve_timeout" and "acknowledge_timeout" etc, this allows to skip response_play by setting value to "null" besides not writing "response_play" field. This is easier for creating multiple services with modules when some have response_play while others don't.

Similar to fields "auto_resolve_timeout" and "acknowledge_timeout" etc, this allows to skip response_play by setting value to "null" besides not writing "response_play" field.
This is easier for creating multiple services with modules when some have response_play while others don't.
Copy link
Contributor

@drastawi drastawi left a comment

Choose a reason for hiding this comment

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

Looks great. @imjaroiswebdev any chance we release this soon? As is, it is very hard to write a terraform module where some services have response plays and some do not, so the 2.6 becomes a breaking change for such modules.

@imjaroiswebdev imjaroiswebdev self-requested a review October 5, 2022 19:19
Copy link
Contributor

@imjaroiswebdev imjaroiswebdev left a comment

Choose a reason for hiding this comment

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

Patch and tests looks great! Thank you @Padorax and @drastawi. I'm proceeding to merge 💪🏽 ✌🏽

@imjaroiswebdev imjaroiswebdev merged commit 962a14b into PagerDuty:master Oct 5, 2022
@s3265176
Copy link

How Do I unset response_play on resource_pagerduty_service then? Need to remove a response_play on a service.

@Padorax
Copy link
Contributor Author

Padorax commented Feb 10, 2023

@s3265176 Before this change, response_play=null will attempt to set the response_play id to string "null", which is not a valid value.
As described in the commit message, this change ensures "null" is accepted as an empty value (i.e., no response_play), which makes a consistent behaviour with many other terraform resources, and easier to use with module.

To unset response_play from a service, you can use response_play=null or respon_play="", both seems to works for me. Can you try and see if you have any issue with the solution?

@s3265176
Copy link

@Padorax I am on 2.10.2
My pagerduty_service is used with a for_each loop
setting response_play = null response_play = "" response_play = " " response_play = "null" all failed to remove the
response play from services.

During terraform plan it did pick up the change
~ response_play = "17420b15-2702-4303-75b7-43e9f5fbdb53" -> "null"

@Padorax
Copy link
Contributor Author

Padorax commented Feb 14, 2023

@s3265176 If the change is detected by "plan", but not actually applied, then seems like it's more broken than we fixed.

@bjor-joh
Copy link

We are experiencing the same behaviour.

@imjaroiswebdev
Copy link
Contributor

Hey @s3265176 @Padorax @bjor-joh This issue should be solved with #680

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.

5 participants