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

monitoring: add CRUD commands for alert policies #1006

Merged
merged 1 commit into from Jul 26, 2021

Conversation

bsnyder788
Copy link
Contributor

@bsnyder788 bsnyder788 commented Jul 19, 2021

No description provided.

Copy link
Member

@andrewsomething andrewsomething left a comment

This is looking great! A couple very minor bits in line. It would be great to get some integration tests as well. See:

https://github.com/digitalocean/doctl/tree/main/integration
https://github.com/digitalocean/doctl/blob/main/CONTRIBUTING.md#integration-tests

I did have trouble testing the create command, but that might be expected right now?

./builds/doctl monitoring alerts create --compare GreaterThan --value 50 --type v1/insights/droplet/cpu --description "Test alert" --enabled  --tags personal
Error: POST https://api.digitalocean.com/v2/monitoring/alerts: 500 (request "4fec9302-c09c-4da5-ab86-7dea6a8ec900") Server Error

commands/monitoring.go Outdated Show resolved Hide resolved
commands/monitoring.go Outdated Show resolved Hide resolved
commands/monitoring.go Outdated Show resolved Hide resolved
commands/monitoring.go Show resolved Hide resolved
commands/monitoring.go Outdated Show resolved Hide resolved
@bsnyder788
Copy link
Contributor Author

bsnyder788 commented Jul 23, 2021

This is looking great! A couple very minor bits in line. It would be great to get some integration tests as well. See:

https://github.com/digitalocean/doctl/tree/main/integration
https://github.com/digitalocean/doctl/blob/main/CONTRIBUTING.md#integration-tests

I did have trouble testing the create command, but that might be expected right now?

./builds/doctl monitoring alerts create --compare GreaterThan --value 50 --type v1/insights/droplet/cpu --description "Test alert" --enabled  --tags personal
Error: POST https://api.digitalocean.com/v2/monitoring/alerts: 500 (request "4fec9302-c09c-4da5-ab86-7dea6a8ec900") Server Error

It looks like you didn't supply any emails or slacks to send the alert to in this one. I will add some better validation so that we can stop it from sending requests without all the required params being passed.

@bsnyder788 bsnyder788 force-pushed the add-alert-policy-CRUD branch 3 times, most recently from c2e7656 to b14ac1b Compare Jul 26, 2021
@bsnyder788
Copy link
Contributor Author

bsnyder788 commented Jul 26, 2021

This is looking great! A couple very minor bits in line. It would be great to get some integration tests as well. See:

https://github.com/digitalocean/doctl/tree/main/integration
https://github.com/digitalocean/doctl/blob/main/CONTRIBUTING.md#integration-tests

I did have trouble testing the create command, but that might be expected right now?

./builds/doctl monitoring alerts create --compare GreaterThan --value 50 --type v1/insights/droplet/cpu --description "Test alert" --enabled  --tags personal
Error: POST https://api.digitalocean.com/v2/monitoring/alerts: 500 (request "4fec9302-c09c-4da5-ab86-7dea6a8ec900") Server Error

I went ahead and added more integration tests.

Copy link
Member

@andrewsomething andrewsomething left a comment

👍 Looks great!

@andrewsomething andrewsomething merged commit 0b5840c into digitalocean:main Jul 26, 2021
7 checks passed
@bsnyder788 bsnyder788 deleted the add-alert-policy-CRUD branch Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants