Skip to content

Commit c71f798

Browse files
authored
Exporter: track removed objects during the Emit phase (#3554)
* Exporter: track removed objects during the `Emit` phase In the current implementation when TF resource data has `.id` set to empty string this means the following: * object doesn't exist on backend - common situation for jobs and DLT pipelines when dependencies are deleted. * Grants or Permissions are empty, so we don't need to generate a code for that. Right now, all such objects are tracked and reported individually via `Ignore` function, but not all objects have it. Also, for big exports, not necessary objects are polluting the internal state without any useful effect. The current PR checks if `.id` is empty during the `Emit` phase, adds them to the list of ignored objects (except permissions & grants), and don't propagate them to the internal state. * Add more tests
1 parent a162a2f commit c71f798

File tree

5 files changed

+39
-24
lines changed

5 files changed

+39
-24
lines changed

exporter/importables.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"strconv"
1414
"strings"
1515

16+
"github.com/databricks/databricks-sdk-go/apierr"
1617
"github.com/databricks/databricks-sdk-go/service/catalog"
1718
"github.com/databricks/databricks-sdk-go/service/compute"
1819
"github.com/databricks/databricks-sdk-go/service/iam"
@@ -234,11 +235,6 @@ var resourcesMap map[string]importable = map[string]importable{
234235
"inst_pool_"+ic.Importables["databricks_instance_pool"].Name(ic, r.Data))
235236
return nil
236237
},
237-
Ignore: func(ic *importContext, r *resource) bool {
238-
isIgnored := r.Data.Get("instance_pool_name") == ""
239-
ic.addIgnoredResource(fmt.Sprintf("databricks_instance_pool. id=%s", r.ID))
240-
return isIgnored
241-
},
242238
},
243239
"databricks_instance_profile": {
244240
Service: "access",
@@ -1511,6 +1507,9 @@ var resourcesMap map[string]importable = map[string]importable{
15111507
notebooksAPI := workspace.NewNotebooksAPI(ic.Context, ic.Client)
15121508
contentB64, err := notebooksAPI.Export(r.ID, ic.notebooksFormat)
15131509
if err != nil {
1510+
if apierr.IsMissing(err) {
1511+
ic.addIgnoredResource(fmt.Sprintf("databricks_notebook. path=%s", r.ID))
1512+
}
15141513
return err
15151514
}
15161515
var fileExtension string
@@ -1558,6 +1557,9 @@ var resourcesMap map[string]importable = map[string]importable{
15581557
notebooksAPI := workspace.NewNotebooksAPI(ic.Context, ic.Client)
15591558
contentB64, err := notebooksAPI.Export(r.ID, "AUTO")
15601559
if err != nil {
1560+
if apierr.IsMissing(err) {
1561+
ic.addIgnoredResource(fmt.Sprintf("databricks_workspace_file. path=%s", r.ID))
1562+
}
15611563
return err
15621564
}
15631565
objectId := r.Data.Get("object_id").(int)
@@ -2609,9 +2611,6 @@ var resourcesMap map[string]importable = map[string]importable{
26092611
}
26102612
return common.StructToData(pList, s, r.Data)
26112613
},
2612-
Ignore: func(ic *importContext, r *resource) bool {
2613-
return r.Data.Get("grant.#").(int) == 0
2614-
},
26152614
Depends: []reference{
26162615
{Path: "catalog", Resource: "databricks_catalog"},
26172616
{Path: "schema", Resource: "databricks_schema"},

exporter/importables_test.go

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,6 @@ func TestInstancePool(t *testing.T) {
100100
err := resourcesMap["databricks_instance_pool"].Import(ic, r)
101101
assert.NoError(t, err)
102102
assert.True(t, ic.testEmits["databricks_permissions[inst_pool_def] (id: /instance-pools/abc)"])
103-
104-
// Check ignore function
105-
assert.True(t, resourcesMap["databricks_instance_pool"].Ignore(ic, r))
106-
assert.Equal(t, 1, len(ic.ignoredResources))
107-
assert.Contains(t, ic.ignoredResources, "databricks_instance_pool. id=abc")
108-
//
109-
d.Set("instance_pool_name", "test")
110-
assert.False(t, resourcesMap["databricks_instance_pool"].Ignore(ic, r))
111-
assert.Equal(t, 1, len(ic.ignoredResources))
112103
}
113104

114105
func TestClusterPolicy(t *testing.T) {
@@ -987,6 +978,30 @@ func TestRepoListFails(t *testing.T) {
987978
})
988979
}
989980

981+
func TestNotebookWorkspaceFileImportNotFound(t *testing.T) {
982+
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
983+
{
984+
ReuseRequest: true,
985+
MatchAny: true,
986+
Status: 404,
987+
Response: apierr.NotFound("nope"),
988+
},
989+
}, func(ctx context.Context, client *common.DatabricksClient) {
990+
ic := importContextForTestWithClient(ctx, client)
991+
err := resourcesMap["databricks_notebook"].Import(ic, &resource{
992+
ID: "/abc",
993+
})
994+
assert.EqualError(t, err, "nope")
995+
assert.Contains(t, ic.ignoredResources, "databricks_notebook. path=/abc")
996+
997+
err = resourcesMap["databricks_workspace_file"].Import(ic, &resource{
998+
ID: "/def",
999+
})
1000+
assert.EqualError(t, err, "nope")
1001+
assert.Contains(t, ic.ignoredResources, "databricks_workspace_file. path=/def")
1002+
})
1003+
}
1004+
9901005
func testGenerate(t *testing.T, fixtures []qa.HTTPFixture, services string, asAdmin bool, cb func(*importContext)) {
9911006
qa.HTTPFixturesApply(t, fixtures, func(ctx context.Context, client *common.DatabricksClient) {
9921007
ic := importContextForTestWithClient(ctx, client)
@@ -2179,9 +2194,6 @@ func TestImportGrants(t *testing.T) {
21792194
err := resourcesMap["databricks_grants"].Import(ic, r)
21802195
assert.NoError(t, err)
21812196

2182-
// Test ignore function
2183-
assert.True(t, resourcesMap["databricks_grants"].Ignore(ic, r))
2184-
21852197
var pList tfcatalog.PermissionsList
21862198
common.DataToStructPointer(r.Data, s, &pList)
21872199
assert.Empty(t, pList.Assignments)

exporter/model.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,12 +343,16 @@ func (r *resource) ImportResource(ic *importContext) {
343343
return pr.ReadContext(ctx, r.Data, ic.Client)
344344
},
345345
fmt.Sprintf("reading %s#%s", r.Resource, r.ID))
346-
if dia != nil {
346+
if dia.HasError() {
347347
log.Printf("[ERROR] Error reading %s#%s: %v", r.Resource, r.ID, dia)
348348
return
349349
}
350350
if r.Data.Id() == "" {
351-
r.Data.SetId(r.ID)
351+
if r.Resource != "databricks_permissions" && r.Resource != "databricks_grants" {
352+
log.Printf("[WARN] %s %s has empty ID because it's deleted or empty", r.Resource, r.ID)
353+
ic.addIgnoredResource(fmt.Sprintf("%s. id=%s", r.Resource, r.ID))
354+
}
355+
return
352356
}
353357
}
354358
r.Name = ic.ResourceName(r)

exporter/util.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1418,7 +1418,7 @@ func generateIgnoreObjectWithoutName(resourceType string) func(ic *importContext
14181418
return func(ic *importContext, r *resource) bool {
14191419
res := (r.Data != nil && r.Data.Get("name").(string) == "")
14201420
if res {
1421-
ic.addIgnoredResource(fmt.Sprintf("%s. ID=%s", resourceType, r.ID))
1421+
ic.addIgnoredResource(fmt.Sprintf("%s. id=%s", resourceType, r.ID))
14221422
}
14231423
return res
14241424
}

exporter/util_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ func TestIgnoreObjectWithEmptyName(t *testing.T) {
406406
require.NotNil(t, ignoreFunc)
407407
assert.True(t, ignoreFunc(ic, r))
408408
require.Equal(t, 1, len(ic.ignoredResources))
409-
assert.Contains(t, ic.ignoredResources, "databricks_volume. ID=vtest")
409+
assert.Contains(t, ic.ignoredResources, "databricks_volume. id=vtest")
410410

411411
d.Set("name", "test")
412412
assert.False(t, ignoreFunc(ic, r))

0 commit comments

Comments
 (0)