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

Fixes Datadogs API quirk described in #3 #16

Merged
merged 2 commits into from
Dec 18, 2017
Merged

Conversation

MrMarvin
Copy link
Contributor

@MrMarvin MrMarvin commented Aug 12, 2017

This fixes #3 / hashicorp/terraform#13784.

Fixed as initially suggested by @kmshultz, thanks for bringing this up!

@grubernaut
Copy link
Contributor

Hey @MrMarvin, thanks for the contribution!
We can, however, add this as a DiffSuppressFunc to this resources schema, instead of adding additional items to the Read function.
https://github.com/hashicorp/terraform/blob/master/helper/schema/schema.go#L59-L66
https://github.com/hashicorp/terraform/blob/master/helper/schema/schema.go#L195-L200

This allows us to fully know that the suppression is happening, without having to find where it's happening inside of the Read function.

@MrMarvin
Copy link
Contributor Author

Thanks @grubernaut for the hint to DiffSuppressFunc. A much cleaner solution!
I just pushed a second commit that reworks the modification to make use of such a function.

@MrMarvin MrMarvin changed the title Fixes Datadogs API quirk described in #3 WIP: Fixes Datadogs API quirk described in #3 Nov 17, 2017
@MrMarvin
Copy link
Contributor Author

setting it to WIP for now, found another issue that needs investigation before merging.

@MrMarvin MrMarvin changed the title WIP: Fixes Datadogs API quirk described in #3 Fixes Datadogs API quirk described in #3 Nov 17, 2017
@MrMarvin
Copy link
Contributor Author

removing 'WIP', double checked and its fine, from my point of view at least.
Is there anything missing?

@MrMarvin
Copy link
Contributor Author

Hi @radeksimko, @grubernaut, is there anything I can do to help get this merged? Please let me know and thanks!

Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

👍 thanks!

@catsby catsby merged commit 914f95a into DataDog:master Dec 18, 2017
@brendanjerwin
Copy link

Whats the release timeline/process? I'd love to get to use this fix!

@catsby
Copy link
Contributor

catsby commented Dec 19, 2017

@brendanjerwin
Copy link

brendanjerwin commented Dec 19, 2017

Awesome! thx. Much quieter now. :)

(I think I found another case of a similar nature though, I'll make a new issue for it: graph.0.style.palette_flip: "false" => "0" )

nitrocode pushed a commit to nitrocode/terraform-provider-datadog that referenced this pull request Nov 3, 2018
* Fixes Datadogs API quirk described in DataDog#3

This fixes DataDog#3.

* moves type modification to DiffSuppressFunc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Datadog monitors - 'metric alert' versus 'query alert'
5 participants