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

feat(alerts): add preset alert logdna_alert as a data source #31

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

david-ly
Copy link
Contributor

Creates a logdna_alert data source which provides support for
remotely fetching information from existing preset alerts. Added
tests, updated the documentation, and included it in the provider.

Semver: minor
Ref: LOG-10590

@david-ly david-ly requested review from darinspivey, jorgebay, LYHuang and a user August 17, 2021 15:54
@david-ly david-ly added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 17, 2021
Copy link
Contributor

@jorgebay jorgebay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome, I've added a question below.

docs/data-sources/logdna_alert.md Show resolved Hide resolved
logdna/data_source_alert.go Show resolved Hide resolved
docs/data-sources/logdna_alert.md Outdated Show resolved Hide resolved
logdna/data_source_alert_test.go Outdated Show resolved Hide resolved
@david-ly david-ly requested a review from tanberry August 18, 2021 17:45
docs/data-sources/logdna_alert.md Show resolved Hide resolved
docs/data-sources/logdna_alert.md Outdated Show resolved Hide resolved
docs/data-sources/logdna_alert.md Show resolved Hide resolved
jorgebay
jorgebay previously approved these changes Aug 31, 2021
Comment on lines 305 to 306
category1 := "DEMOCATEGORY1"
category2 := "DemoCategory2"
category1 := "category1"
category2 := "category2"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changing these will require changes on the integration account in production - is there a reason for switching these?

also iirc these were set up with mixed casing specifically for testing that scenario

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdeltito nah this was just a byproduct of troubleshooting the tests, I was using the service key for the LogDNA Canary account in production instead of the MDT-terraform account which is why the Jenkins builds weren't passing. This took me an unfortunate amount of time to figure out since the local testing didn't 100% align with the build results.

Reverted my changes since the tests were already correct initially.

Copy link
Member

@mdeltito mdeltito Sep 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of the MDT-terraform account

@david-ly the CI account is named:

Terraform Plugin Provider Tests (terraform-provider-logdna)

we should get that into the docs and make sure it's available to everyone (edit: done, opened it up)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access control is wide open, I've confirmed. Looks like just a doc issue now.

Allow members with an @logdna.com email to join this organization without a request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, thanks for clarifying. I'll add this to the docs in a future PR

@david-ly david-ly force-pushed the david-ly/LOG-10590 branch 4 times, most recently from 053c1b5 to ed0cf7a Compare September 23, 2021 00:32
Creates a `logdna_alert` data source which provides support for
remotely fetching information from existing preset alerts. Added
tests, updated the documentation, and included it in the provider.

Semver: minor
Ref: LOG-10590
@david-ly david-ly dismissed tanberry’s stale review September 30, 2021 21:25

Outdated and currently blocking merge

@david-ly david-ly requested review from a team September 30, 2021 21:25
@david-ly david-ly merged commit dfb2ce1 into main Oct 1, 2021
@david-ly david-ly deleted the david-ly/LOG-10590 branch October 1, 2021 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants