Skip to content

Commit

Permalink
[Fix] Set ID for online table resource if creation succeeds but it is…
Browse files Browse the repository at this point in the history
…n't available yet (#4072)

## Changes
<!-- Summary of your changes that are easy to understand -->
We should set the id right after creation and before waiting for online
table to be available. This is because in case when online table isn't
available, we still should have that resource in the state.

Also the timeout has been increased to 2x (I am going to following up
with online tables team for suitable timeout but since we have to do a
release, going ahead with small time increase should be good)

Note: We should add setting id right after creation for similar
resources to CONTRIBUTING guide (which I will do in a separate PR)

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->
Added unit test to check that pathway, id is set (which wasn't the case
before)
- [ ] `make test` run locally
- [ ] relevant change in `docs/` folder
- [ ] covered with integration tests in `internal/acceptance`
- [ ] relevant acceptance tests are passing
- [ ] using Go SDK

---------

Co-authored-by: Miles Yucht <miles@databricks.com>
  • Loading branch information
tanmay-db and mgyucht authored Oct 4, 2024
1 parent 7d0491f commit 60b8a6f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 5 deletions.
7 changes: 4 additions & 3 deletions catalog/resource_online_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

const onlineTableDefaultProvisionTimeout = 45 * time.Minute
const onlineTableDefaultProvisionTimeout = 90 * time.Minute

func waitForOnlineTableCreation(w *databricks.WorkspaceClient, ctx context.Context, onlineTableName string) error {
return retry.RetryContext(ctx, onlineTableDefaultProvisionTimeout, func() *retry.RetryError {
Expand Down Expand Up @@ -80,13 +80,14 @@ func ResourceOnlineTable() common.Resource {
if err != nil {
return err
}
// Note: We should set the id right after creation and before waiting for online table to be available.
// If the resource creation timeout is exceeded while waiting for the online table to be ready, this ensures the online table is persisted in the state.
d.SetId(res.Name)
// this should be specified in the API Spec - filed a ticket to add it
err = waitForOnlineTableCreation(w, ctx, res.Name)
if err != nil {

return err
}
d.SetId(res.Name)
return nil
},
Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
Expand Down
7 changes: 5 additions & 2 deletions catalog/resource_online_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/databricks/databricks-sdk-go/service/catalog"
"github.com/databricks/terraform-provider-databricks/qa"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)

Expand Down Expand Up @@ -108,7 +109,7 @@ func TestOnlineTableCreate_ErrorInWait(t *testing.T) {
},
Status: &catalog.OnlineTableStatus{DetailedState: catalog.OnlineTableStateOfflineFailed},
}
qa.ResourceFixture{
d, err := qa.ResourceFixture{
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
e := w.GetMockOnlineTablesAPI().EXPECT()
e.Create(mock.Anything, catalog.CreateOnlineTableRequest{
Expand All @@ -124,7 +125,9 @@ func TestOnlineTableCreate_ErrorInWait(t *testing.T) {
Resource: ResourceOnlineTable(),
HCL: onlineTableHcl,
Create: true,
}.ExpectError(t, "online table status returned OFFLINE_FAILED for online table: main.default.online_table")
}.Apply(t)
qa.AssertErrorStartsWith(t, err, "online table status returned OFFLINE_FAILED for online table: main.default.online_table")
assert.Equal(t, "main.default.online_table", d.Id())
}

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

0 comments on commit 60b8a6f

Please sign in to comment.