search

PagerDuty / terraform-provider-pagerduty

Terraform PagerDuty provider
https://www.terraform.io/docs/providers/pagerduty/
Mozilla Public License 2.0
204 stars 208 forks source link

feat: improve PD service time window validation to accept 86400 as a valid value #876

Closed boneff closed 3 weeks ago

boneff commented 1 month ago

Aims to fix: https://github.com/PagerDuty/terraform-provider-pagerduty/issues/849

imjaroiswebdev commented 1 month ago

Hi @boneff thank you for your contribution, is a quite complete PR, which also includes the necessary acceptance tests. We appreciate it. However this input value is not currently supported by pagerduty_service.alert_grouping_parameters.0.config.0.time_window as I stated here.

Therefore, I'm proceeding to close this PR for the moment, if the input range gets updated to support what this PR is aiming to add, then We'll considere it as an update to bring support for the new input range.

imjaroiswebdev commented 3 weeks ago

Hi @boneff you were right about this update, because when I reviewed this improvement, I checked the valid input range for Intelligent Alert Grouping, which is limited to 3600 max. However, this update targets Content Based Alert Grouping input range, which indeed supports 300 <= time_window <= 3600 or 86400(i.e. 24 hours).

So, I'm reopening and reviewing it again. Sorry for the mixup btw 😓

boneff commented 2 weeks ago

Thank you adding this improvement @boneff. I took the liberty of rebasing and updating the documentation.

Thank you for reconsidering @imjaroiswebdev! Any idea when that change would be released ?

imjaroiswebdev commented 2 weeks ago

Hi @boneff is already available in version latest version of PagerDuty TF provider, it was released more precisely in version v3.14.0