Skip to content

Commit

Permalink
[Internal] Use generic error for missing clusters (#3938)
Browse files Browse the repository at this point in the history
## Changes
<!-- Summary of your changes that are easy to understand -->
We use generic errors specified as `ErrResourceDoesNotExist` if a
cluster is in invalid state or missing.

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->
Unit tests
- [ ] `make test` run locally
- [ ] relevant change in `docs/` folder
- [ ] covered with integration tests in `internal/acceptance`
- [ ] relevant acceptance tests are passing
- [ ] using Go SDK
  • Loading branch information
tanmay-db authored Aug 22, 2024
1 parent f833cf2 commit ff4974b
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 12 deletions.
6 changes: 2 additions & 4 deletions clusters/clusters_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,14 +730,12 @@ func wrapMissingClusterError(err error, id string) error {
// as is in the longer term, so that this keeps working.
if apiErr.ErrorCode == "INVALID_STATE" {
log.Printf("[WARN] assuming that cluster is removed on backend: %s", apiErr)
apiErr.StatusCode = 404
return apiErr
return databricks.ErrResourceDoesNotExist
}
// fix non-compliant error code
if strings.Contains(apiErr.Message,
fmt.Sprintf("Cluster %s does not exist", id)) {
apiErr.StatusCode = 404
return apiErr
return databricks.ErrResourceDoesNotExist
}
return err
}
Expand Down
8 changes: 3 additions & 5 deletions clusters/clusters_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ package clusters

import (
"context"
"errors"
"fmt"

// "reflect"

"testing"

"github.com/databricks/databricks-sdk-go"
"github.com/databricks/databricks-sdk-go/apierr"
"github.com/databricks/databricks-sdk-go/service/compute"
"github.com/databricks/terraform-provider-databricks/common"
Expand Down Expand Up @@ -1158,15 +1158,13 @@ func TestWrapMissingClusterError(t *testing.T) {
assert.EqualError(t, wrapMissingClusterError(fmt.Errorf("x"), "abc"), "x")
assert.EqualError(t, wrapMissingClusterError(&apierr.APIError{
Message: "Cluster abc does not exist",
}, "abc"), "Cluster abc does not exist")
}, "abc"), databricks.ErrResourceDoesNotExist.Error())
}

func TestExpiredClusterAssumedAsRemoved(t *testing.T) {
err := wrapMissingClusterError(&apierr.APIError{
ErrorCode: "INVALID_STATE",
Message: "Cannot access cluster X that was terminated or unpinned more than Y days ago.",
}, "X")
var ae *apierr.APIError
assert.True(t, errors.As(err, &ae))
assert.Equal(t, 404, ae.StatusCode)
assert.EqualError(t, err, databricks.ErrResourceDoesNotExist.Error())
}
5 changes: 2 additions & 3 deletions clusters/resource_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ func TestResourceClusterRead(t *testing.T) {
}

func TestResourceClusterRead_NotFound(t *testing.T) {
_, err := qa.ResourceFixture{
qa.ResourceFixture{
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Expand All @@ -531,8 +531,7 @@ func TestResourceClusterRead_NotFound(t *testing.T) {
Read: true,
Removed: true,
ID: "abc",
}.Apply(t)
qa.AssertErrorStartsWith(t, err, "Cluster abc does not exist")
}.ApplyNoError(t)
}

func TestResourceClusterRead_Error(t *testing.T) {
Expand Down

0 comments on commit ff4974b

Please sign in to comment.