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

Merge some more PRs #153

Closed
wants to merge 5 commits into from
Closed

Merge some more PRs #153

wants to merge 5 commits into from

Conversation

paultyng
Copy link
Contributor

Includes #136, #141, and dependency updates.

paultyng and others added 5 commits July 10, 2019 21:11
NRQL alert conditions have 3 different types: static, outlier and
baseline. 'static' is the default type, while 'baseline' is not yet
supported by the REST API.

The 'outlier' type requires two additional fields: 'expected_groups' (an
integer) and 'ignore_overlap' (a boolean).
The API response for the creation of an AlertNrqlCondition with a type
other than the default (static), such as an outlier condition, does not
contain the `value_function` JSON field, and thus the struct's
`ValueFunction` is set to the default string value (i.e. an empty
string).

That discrepancy creates a (virtual) difference between the plan and the
state which can never be resolved, and thus `terraform apply` would seem
to fail despite the changes being correctly applied.

The approach taken here is setting default values for absent fields when
they're absent from the response, which maintains backward compatibility
with existing plans.

An alternative approach where the default value was removed from the
field definition in AlertNrqlCondition was also considered but discarded
because it created a breaking change.
* API allows any positive integer
* Previously limited to 1, 2, 3, 4, 5, 10, 15, 30, 60, or 120
@ghost ghost added size/XXL dependencies Changes to dependencies labels Jul 11, 2019
@i-wilson
Copy link

Any updates on merging this one? I'd love to use outlier NRQL conditions

@sanderblue
Copy link
Contributor

Addressed in #141, #144, and #178 (all merged).

@sanderblue sanderblue closed this Nov 6, 2019
@sanderblue sanderblue deleted the wip branch November 6, 2019 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Changes to dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants