Add New Relic Infrastructure alert condition support #30
Conversation
|
Hey Paul, thanks for the PR! I have contributor access to There's a separate issue open that discusses just merging |
|
Hello! The only reason I forked I am happy to revisit that challenge if it's the preferred path going forward. I will see what I can do when I have some spare time, thanks for taking a look. |
|
Im realy interested in this option. Seeing that I really want any of our new relic config to be in terraform code. |
|
@smithclay Any progress on this? Would you be willing to merge this PR to a 1.0.1 release with a view to refactor later? It'd be really great to have this in official mainline now that Servers has been fully EOL by NewRelic. |
|
I unfortunately haven't had time to refactor the API part of this as I've also been dealing with the Servers EOL. I will hopefully have time to revisit this by early next week. |
|
Attempted to move the Infra API CLI stuff into the original |
|
Many thanks @paul91 really appreciate your efforts here! :) |
|
paultyng/go-newrelic#15 was merged. I will begin refactoring this to make use of it and will resubmit once ready for another review. |
|
That's great Paul thanks, again very grateful for your work here and look
forward to seeing it! :)
|
paul91
force-pushed
the
feature/newrelic-infra-conditions
branch
from
9e5647f to
a2e0202
Compare
|
PR updated to be based on the changes to |
|
@smithclay Another nudge here :) We've got a bunch of Infrastructure alert-condition stubs set up in Terraform currently that are crying out for the new provider version 🤞 |
| @@ -84,7 +83,7 @@ func TestAccNewRelicNrqlAlertCondition_Basic(t *testing.T) { | |||
| // TODO: func_ TestAccNewRelicNrqlAlertCondition_Multi(t *testing.T) { | |||
There was a problem hiding this comment.
Finished fest case for this one would be a nice-to-have :)
There was a problem hiding this comment.
I will take a look at it
There was a problem hiding this comment.
I am actually unsure what the intention of that test case was as it isn't related to my changes or Infra. It looks like it was also copy pasted from a different test file: https://github.com/terraform-providers/terraform-provider-newrelic/blob/master/newrelic/resource_newrelic_alert_condition_test.go#L111
There was a problem hiding this comment.
Fair enough (see the some other TODOs as well). Approving this and merging after running the acceptance tests.
|
This has now been released in provider version v1.0.1. @paul91 Thanks very much for your contribution! |
…ditions Add New Relic Infrastructure alert condition support
My attempt at adding support for New Relic Infrastructure alert conditions. Adding this was complicated by the fact that New Relic has a separate endpoint for anything Infrastructure related.
I basically just forked paultyng/go-newrelic into paul91/go-newrelic-infra and made it Infrastructure specific. This allowed me to export it as a secondary client for the provider.
Happy to fix/change whatever. Thanks!