Skip to content

Commit

Permalink
moved FindVolumeInBundle
Browse files Browse the repository at this point in the history
  • Loading branch information
shreyas-goenka committed Dec 30, 2024
1 parent 2a590b8 commit 56cd37b
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 93 deletions.
27 changes: 26 additions & 1 deletion bundle/config/validate/validate_artifact_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (
"strings"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/libraries"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/dyn/dynvar"
"github.com/databricks/databricks-sdk-go/apierr"
"github.com/databricks/databricks-sdk-go/service/catalog"
)
Expand Down Expand Up @@ -50,6 +52,29 @@ func extractVolumeFromPath(artifactPath string) (string, string, string, error)
return catalogName, schemaName, volumeName, nil
}

func findVolumeInBundle(r config.Root, catalogName, schemaName, volumeName string) (dyn.Path, []dyn.Location, bool) {
volumes := r.Resources.Volumes
for k, v := range volumes {
if v.CatalogName != catalogName || v.Name != volumeName {
continue
}
// UC schemas can be defined in the bundle itself, and thus might be interpolated
// at runtime via the ${resources.schemas.<name>} syntax. Thus we match the volume
// definition if the schema name is the same as the one in the bundle, or if the
// schema name is interpolated.
// We only have to check for ${resources.schemas...} references because any
// other valid reference (like ${var.foo}) would have been interpolated by this point.
p, ok := dynvar.PureReferenceToPath(v.SchemaName)
isSchemaDefinedInBundle := ok && p.HasPrefix(dyn.Path{dyn.Key("resources"), dyn.Key("schemas")})
if v.SchemaName != schemaName && !isSchemaDefinedInBundle {
continue
}
pathString := fmt.Sprintf("resources.volumes.%s", k)
return dyn.MustPathFromString(pathString), r.GetLocations(pathString), true
}
return nil, nil, false
}

func (v *validateArtifactPath) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.Diagnostics {
// We only validate UC Volumes paths right now.
if !libraries.IsVolumesPath(rb.Config().Workspace.ArtifactPath) {
Expand Down Expand Up @@ -79,7 +104,7 @@ func (v *validateArtifactPath) Apply(ctx context.Context, rb bundle.ReadOnlyBund
return wrapErrorMsg(fmt.Sprintf("cannot access volume %s: %s", volumeFullName, err))
}
if errors.Is(err, apierr.ErrNotFound) {
path, locations, ok := libraries.FindVolumeInBundle(rb.Config(), catalogName, schemaName, volumeName)
path, locations, ok := findVolumeInBundle(rb.Config(), catalogName, schemaName, volumeName)
if !ok {
return wrapErrorMsg(fmt.Sprintf("volume %s does not exist", volumeFullName))
}
Expand Down
65 changes: 65 additions & 0 deletions bundle/config/validate/validate_artifact_path_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,68 @@ func TestValidateArtifactPathWithInvalidPaths(t *testing.T) {
}})
}
}

func TestFindVolumeInBundle(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Resources: config.Resources{
Volumes: map[string]*resources.Volume{
"foo": {
CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
CatalogName: "main",
Name: "my_volume",
SchemaName: "my_schema",
},
},
},
},
},
}

bundletest.SetLocation(b, "resources.volumes.foo", []dyn.Location{
{
File: "volume.yml",
Line: 1,
Column: 2,
},
})

// volume is in DAB.
path, locations, ok := findVolumeInBundle(b.Config, "main", "my_schema", "my_volume")
assert.True(t, ok)
assert.Equal(t, []dyn.Location{{
File: "volume.yml",
Line: 1,
Column: 2,
}}, locations)
assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)

// wrong volume name
_, _, ok = findVolumeInBundle(b.Config, "main", "my_schema", "doesnotexist")
assert.False(t, ok)

// wrong schema name
_, _, ok = findVolumeInBundle(b.Config, "main", "doesnotexist", "my_volume")
assert.False(t, ok)

// wrong catalog name
_, _, ok = findVolumeInBundle(b.Config, "doesnotexist", "my_schema", "my_volume")
assert.False(t, ok)

// schema name is interpolated but does not have the right prefix. In this case
// we should not match the volume.
b.Config.Resources.Volumes["foo"].SchemaName = "${foo.bar.baz}"
_, _, ok = findVolumeInBundle(b.Config, "main", "my_schema", "my_volume")
assert.False(t, ok)

// schema name is interpolated.
b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema.name}"
path, locations, ok = findVolumeInBundle(b.Config, "main", "valuedoesnotmatter", "my_volume")
assert.True(t, ok)
assert.Equal(t, []dyn.Location{{
File: "volume.yml",
Line: 1,
Column: 2,
}}, locations)
assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
}
27 changes: 0 additions & 27 deletions bundle/libraries/filer_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,10 @@ package libraries

import (
"context"
"fmt"
"path"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/dyn/dynvar"
"github.com/databricks/cli/libs/filer"
)

Expand All @@ -19,26 +15,3 @@ func filerForVolume(ctx context.Context, b *bundle.Bundle) (filer.Filer, string,
f, err := filer.NewFilesClient(w, uploadPath)
return f, uploadPath, diag.FromErr(err)
}

func FindVolumeInBundle(r config.Root, catalogName, schemaName, volumeName string) (dyn.Path, []dyn.Location, bool) {
volumes := r.Resources.Volumes
for k, v := range volumes {
if v.CatalogName != catalogName || v.Name != volumeName {
continue
}
// UC schemas can be defined in the bundle itself, and thus might be interpolated
// at runtime via the ${resources.schemas.<name>} syntax. Thus we match the volume
// definition if the schema name is the same as the one in the bundle, or if the
// schema name is interpolated.
// We only have to check for ${resources.schemas...} references because any
// other valid reference (like ${var.foo}) would have been interpolated by this point.
p, ok := dynvar.PureReferenceToPath(v.SchemaName)
isSchemaDefinedInBundle := ok && p.HasPrefix(dyn.Path{dyn.Key("resources"), dyn.Key("schemas")})
if v.SchemaName != schemaName && !isSchemaDefinedInBundle {
continue
}
pathString := fmt.Sprintf("resources.volumes.%s", k)
return dyn.MustPathFromString(pathString), r.GetLocations(pathString), true
}
return nil, nil, false
}
65 changes: 0 additions & 65 deletions bundle/libraries/filer_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,71 +22,6 @@ import (
"github.com/stretchr/testify/require"
)

func TestFindVolumeInBundle(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Resources: config.Resources{
Volumes: map[string]*resources.Volume{
"foo": {
CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
CatalogName: "main",
Name: "my_volume",
SchemaName: "my_schema",
},
},
},
},
},
}

bundletest.SetLocation(b, "resources.volumes.foo", []dyn.Location{
{
File: "volume.yml",
Line: 1,
Column: 2,
},
})

// volume is in DAB.
path, locations, ok := FindVolumeInBundle(b.Config, "main", "my_schema", "my_volume")
assert.True(t, ok)
assert.Equal(t, []dyn.Location{{
File: "volume.yml",
Line: 1,
Column: 2,
}}, locations)
assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)

// wrong volume name
_, _, ok = FindVolumeInBundle(b.Config, "main", "my_schema", "doesnotexist")
assert.False(t, ok)

// wrong schema name
_, _, ok = FindVolumeInBundle(b.Config, "main", "doesnotexist", "my_volume")
assert.False(t, ok)

// wrong catalog name
_, _, ok = FindVolumeInBundle(b.Config, "doesnotexist", "my_schema", "my_volume")
assert.False(t, ok)

// schema name is interpolated but does not have the right prefix. In this case
// we should not match the volume.
b.Config.Resources.Volumes["foo"].SchemaName = "${foo.bar.baz}"
_, _, ok = FindVolumeInBundle(b.Config, "main", "my_schema", "my_volume")
assert.False(t, ok)

// schema name is interpolated.
b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema.name}"
path, locations, ok = FindVolumeInBundle(b.Config, "main", "valuedoesnotmatter", "my_volume")
assert.True(t, ok)
assert.Equal(t, []dyn.Location{{
File: "volume.yml",
Line: 1,
Column: 2,
}}, locations)
assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
}

func TestFilerForVolumeForErrorFromAPI(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Expand Down

0 comments on commit 56cd37b

Please sign in to comment.