Skip to content

Commit

Permalink
Do not replace pipeline libraries if there are no matches for pattern (
Browse files Browse the repository at this point in the history
…#1021)

## Changes
If there are no matches when doing Glob call for pipeline library
defined, leave the entry as is.
The next mutators in the chain will detect that file is missing and the
error will be more user friendly.


Before the change

```
Starting resource deployment
Error: terraform apply: exit status 1

Error: cannot create pipeline: libraries must contain at least one element
```

After

```
Error: notebook ./non-existent not found
```


## Tests
Added regression unit tests
  • Loading branch information
andrewnester committed Nov 29, 2023
1 parent 5431174 commit 833746c
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 1 deletion.
5 changes: 5 additions & 0 deletions bundle/config/mutator/expand_pipeline_glob_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ func (m *expandPipelineGlobPaths) Apply(_ context.Context, b *bundle.Bundle) err
return err
}

if len(matches) == 0 {
expandedLibraries = append(expandedLibraries, *library)
continue
}

for _, match := range matches {
m, err := filepath.Rel(dir, match)
if err != nil {
Expand Down
8 changes: 7 additions & 1 deletion bundle/config/mutator/expand_pipeline_glob_paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ func TestExpandGlobPathsInPipelines(t *testing.T) {
Path: "/Repos/somerepo/test.ipynb",
},
},
{
Notebook: &pipelines.NotebookLibrary{
Path: "./non-existent.ipynb",
},
},
},
},
},
Expand All @@ -98,7 +103,7 @@ func TestExpandGlobPathsInPipelines(t *testing.T) {
require.NoError(t, err)

libraries := b.Config.Resources.Pipelines["pipeline"].Libraries
require.Len(t, libraries, 10)
require.Len(t, libraries, 11)

// Making sure glob patterns are expanded correctly
require.True(t, containsNotebook(libraries, filepath.Join("test", "test2.ipynb")))
Expand All @@ -117,6 +122,7 @@ func TestExpandGlobPathsInPipelines(t *testing.T) {
// Making sure other libraries are not replaced
require.True(t, containsJar(libraries, "./*.jar"))
require.True(t, containsMaven(libraries, "org.jsoup:jsoup:1.7.2"))
require.True(t, containsNotebook(libraries, "./non-existent.ipynb"))
}

func containsNotebook(libraries []pipelines.PipelineLibrary, path string) bool {
Expand Down
12 changes: 12 additions & 0 deletions bundle/tests/bundle/pipeline_glob_paths/databricks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
bundle:
name: pipeline_glob_paths

resources:
pipelines:
nyc_taxi_pipeline:
name: "nyc taxi loader"
libraries:
- notebook:
path: ./dlt/*
- notebook:
path: ./non-existent
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Databricks notebook source

print("Hello from notebook!")
40 changes: 40 additions & 0 deletions bundle/tests/bundle/pipeline_glob_paths_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package bundle

import (
"context"
"testing"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/mutator"
"github.com/databricks/cli/bundle/phases"
"github.com/databricks/cli/cmd/root"
"github.com/databricks/databricks-sdk-go/service/iam"
"github.com/stretchr/testify/require"
)

func TestExpandPipelineGlobPathsWithNonExistent(t *testing.T) {
ctx := context.Background()
ctx = root.SetWorkspaceClient(ctx, nil)

b, err := bundle.Load(ctx, "./pipeline_glob_paths")
require.NoError(t, err)

err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...))
require.NoError(t, err)
b.Config.Bundle.Target = "default"

b.Config.Workspace.CurrentUser = &config.User{User: &iam.User{UserName: "user@domain.com"}}
b.WorkspaceClient()

m := phases.Initialize()
err = bundle.Apply(ctx, b, m)
require.Error(t, err)
require.ErrorContains(t, err, "notebook ./non-existent not found")

require.Equal(
t,
b.Config.Resources.Pipelines["nyc_taxi_pipeline"].Libraries[0].Notebook.Path,
"/Users/user@domain.com/.bundle/pipeline_glob_paths/default/files/dlt/nyc_taxi_loader",
)
}

0 comments on commit 833746c

Please sign in to comment.