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

GetIncludeTags() defaults to false while API doc says the default is true #237

Open
pdecat opened this issue May 9, 2019 · 1 comment
Open

Comments

@pdecat
Copy link

pdecat commented May 9, 2019

The generated code for GetIncludeTags() returns false by default: https://github.com/zorkian/go-datadog-api/blob/master/datadog-accessors.go#L10560

However, the API reference documentation says the default value of include_tags is true:

include_tags a boolean indicating whether notifications from this monitor automatically inserts its triggering tags into the title. Default: True

cf. https://docs.datadoghq.com/api/?lang=python#create-a-monitor

As a user of the library, this does not feel natural and can cause confusion.
The work-around is to always use the GetIncludeTagsOk() variant to only use the value if it is actually set.

Note: integration tests where recently updated to define IncludeTags in all cases:
https://github.com/zorkian/go-datadog-api/pull/210/files#diff-e4839e926367a711a23f8e2642ce0a3bR161 , however the datadog API currently does not return that field if it is not explicitly set: https://github.com/terraform-providers/terraform-provider-datadog/issues/94#issuecomment-482557171

@bkabrda
Copy link
Collaborator

bkabrda commented Feb 3, 2020

Hey 👋 thanks for opening the issue. In terms of getting the GetIncludeTags function to return something else, that would be pretty complicated - this is an autogenerated accessor. The semantics of the autogenerated accessors is not actually "what is the API side default?", but "what is currently set on the structure?". It's unlikely that we'd want to change this, as it might break expectations that other users have about how the accessors work.

In terms of the terraform PR comment, we managed to get that fixed as mentioned in a followup comment to that.

I don't see anything actionable to do about this issue and lean towards closing it. Do you have any other related issues/notes that should be taken into account here?

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

No branches or pull requests

2 participants