zorkian / go-datadog-api

A Go implementation of the Datadog API.
BSD 3-Clause "New" or "Revised" License
183 stars 156 forks source link

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

Open pdecat opened 5 years ago

pdecat commented 5 years ago

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 commented 4 years ago

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?