-
Notifications
You must be signed in to change notification settings - Fork 216
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
r/pagerduty_schedule: Don't read back start
value for schedule layers.
#35
Conversation
👏 thanks for the super quick response on this issue! |
Is there something holding this back from being merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Is there any PagerDuty API docs for this behavior? At a minimum we need to document this behavior in https://www.terraform.io/docs/providers/pagerduty/r/schedule.html . If docs exist, let's link to them.
Also, if the updated/new start
value from the API has any kind of significance or nice to know, would it be worth storing that value in a Computed
attribute?
Requested change:
- document behavior in our docs, link to PagerDuty API docs if they exist
Possibly nice to have but not blocker:
- save the always updating value in a
Computed
attributed, if it's useful
The PagerDuty API doesn't really document the resources it exposes or any of their quirks, just the generic API methods available. https://v2.developer.pagerduty.com/docs/endpoints I poked around the API and I believe it's not that the API always returns a new value, but rather, the start time is getting updated every time terraform updates the schedule. It may actually be that there's a way to prevent that from happening, but I'm not sure right now. |
@2rs2ts - Your observation is correct. The @catsby - Unfortunately this behavior isn't documented anywhere, but I made an effort describing the behavior in the resource documentation. I don't think the updated value we get back from the API is of any real value, but it would in that case make more sense to store that updated value under a different name such as: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for investigating and documenting 👍
Nice @heimweh! |
This PR fixes a bug found in
pagerduty_schedule
(described in #34) related tostart
.The PagerDuty API always returns a new
start
value,which in turn causes Terraform to get a diff on every plan.
This PR should resolve that, by ignoring whatever value we get back from the API.