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

Fix: add missing inputs to datadog_monitor resource, update datadog_monitors var to use map(any) #86

Conversation

mldevpants
Copy link

@mldevpants mldevpants commented Feb 7, 2024

what

The main resource for "datadog_monitor" was missing several inputs, even the examples include those inputs but they all set to "null" when applied or plan.

  • main.tf to include the missing inputs with lookup function
  • datadog_monitors variable was updated to use map(any) with objects in description
  • no README reference was found to update
  • examples have all these inputs, no need to add

why

Priority and others are essential part of the monitors to be used in some cases

references

#85

@mldevpants mldevpants requested review from a team as code owners February 7, 2024 14:04
@mldevpants
Copy link
Author

mldevpants commented Feb 7, 2024

the change should should be back compatible since the var that was updated can include any map, while the resource uses lookup with default to null for the new inputs, if few of the original inputs that don't use lookup are missing the terraform will through an error either way.
Meaning it can be a hotifx version update only

@mldevpants
Copy link
Author

additionally, the code was tested with datadog provider versions 3.35 and 3.34

@Nuru
Copy link
Contributor

Nuru commented Feb 8, 2024

@mldevpants Thank you for this contribution.

Sorry for the timing: We will have an updated version of the Datadog monitor support out very soon now. We are taking a different approach moving forward, so I am going to close this PR as not compatible with our upcoming changes.

FYI, map(any) does not work, because although any can be anything, it has to be something. Meaning that even though the values of a map(any) can be of any type, they have to all be of the same type. That is not a restriction we want to apply, so we need to stick with a type of any.

@mldevpants
Copy link
Author

mldevpants commented Feb 8, 2024

Thanks @Nuru for letting me know.
Good point for map(any), didn't think of it yesterday, I was helping out with for some team.
Do you have an ETA for the new one?
Hmmm, I was creating an internal module and saw this one; probably I'll stick to finish internal then.

@Nuru
Copy link
Contributor

Nuru commented Feb 9, 2024

@mldevpants most inputs were added in #87 which has been released as v1.4.0. The remaining few left out are in #89 and should be released very soon.

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.

2 participants