-
Notifications
You must be signed in to change notification settings - Fork 389
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
[dashboards] Add resources for log, apm and process query in legacy dashboards #272
[dashboards] Add resources for log, apm and process query in legacy dashboards #272
Conversation
253a5ff
to
48d1b66
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some inline comments. Additionally:
- The read part is missing in screenboards resource. To verify this is working as expected, we need to run
import
command https://www.terraform.io/docs/providers/datadog/r/screenboard.html#import and verify the internal state contains the correct values. - Let's add some tests for the new params.
a97405d
to
d504d65
Compare
Hey @jbenais thanks for the PR! The golang client was released + bumped in this provider. Can you rebase from master and we can work on getting this reviewed? |
6a97922
to
bd9f4e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for the timeboard/screenbaord features! Awesome stuff. Left a few inline comments/questions, let me know what you think.
7b655c3
to
3b451db
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Looks good to me. Ran some manual applies and seems to work well there.
Left a really small comment about what looks like a typo, should be a super quick thing. Lets get a release going on the go client so we can remove the vendor
changes and get this merged 🎉
a515ea6
to
5afda25
Compare
This functionality relies on a change in the Go client that is in a merged PR: zorkian/go-datadog-api#251