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

Be nicer when a dashboard is deleted from grafana #7

Merged
merged 1 commit into from
Oct 10, 2017

Conversation

roidelapluie
Copy link
Contributor

Signed-off-by: Julien Pivotto roidelapluie@inuits.eu

Copy link
Contributor

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

This looks reasonable (assuming there is no cleaner way to catch 404 other than string matching it).

Just one question - do you mind adding a test for this? We have some tests for such behaviour elsewhere, e.g. https://github.com/terraform-providers/terraform-provider-aws/blob/b8d4e1570fc43a2acee6b6e47f63c9db6b067fa2/aws/resource_aws_cloudwatch_log_stream_test.go#L33-L52

@roidelapluie
Copy link
Contributor Author

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v  -timeout 120m
?   	github.com/terraform-providers/terraform-provider-grafana	[no test files]
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccAlertNotification_basic
request to  http://admin:admin@127.0.0.1:3000/api/alert-notifications with body data {"name":"terraform-acc-test","type":"email","isDefault":false,"settings":{"addresses":"foo@bar.test","uploadImage":"false"}}
request to  http://admin:admin@127.0.0.1:3000/api/alert-notifications/4 with no body data
request to  http://admin:admin@127.0.0.1:3000/api/alert-notifications/4 with no body data
request to  http://admin:admin@127.0.0.1:3000/api/alert-notifications/4 with no body data
request to  http://admin:admin@127.0.0.1:3000/api/alert-notifications/4 with no body data
request to  http://admin:admin@127.0.0.1:3000/api/alert-notifications/4 with no body data
request to  http://admin:admin@127.0.0.1:3000/api/alert-notifications/4 with no body data
--- FAIL: TestAccAlertNotification_basic (0.22s)
	testing.go:499: Error destroying resource! WARNING: Dangling resources
		may exist. The full state and error is shown below.
		
		Error: Check failed: alert-notification still exists
		
		State: <no state>
=== RUN   TestAccDashboard_basic
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db with body data {"dashboard":{"title":"Terraform Acceptance Test","version":0},"overwrite":false}
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
--- PASS: TestAccDashboard_basic (0.23s)
=== RUN   TestAccDashboard_disappear
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db with body data {"dashboard":{"title":"Terraform Acceptance Test","version":0},"overwrite":false}
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
request to  http://admin:admin@127.0.0.1:3000/api/dashboards/db/terraform-acceptance-test with no body data
--- PASS: TestAccDashboard_disappear (0.19s)
=== RUN   TestAccDataSource_basic
request to  http://admin:admin@127.0.0.1:3000/api/datasources with body data {"name":"terraform-acc-test","type":"influxdb","url":"http://terraform-acc-test.invalid/","access":"proxy","database":"terraform-acc-test","user":"terraform_user","password":"terraform_password","isDefault":false,"basicAuth":false}
request to  http://admin:admin@127.0.0.1:3000/api/datasources/4 with no body data
request to  http://admin:admin@127.0.0.1:3000/api/datasources/4 with no body data
request to  http://admin:admin@127.0.0.1:3000/api/datasources/4 with no body data
request to  http://admin:admin@127.0.0.1:3000/api/datasources/4 with no body data
request to  http://admin:admin@127.0.0.1:3000/api/datasources/4 with no body data
request to  http://admin:admin@127.0.0.1:3000/api/datasources/4 with no body data
--- PASS: TestAccDataSource_basic (0.22s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-grafana/grafana	0.872s

@roidelapluie
Copy link
Contributor Author

Failure is unrelated

Copy link
Contributor

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks.

@radeksimko radeksimko merged commit 5940399 into grafana:master Oct 10, 2017
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.

2 participants