Expand library globs relative to the sync root (#1756)

## Changes

Library glob expansion happens during deployment. Before that, all
entries that refer to local paths in resource definitions are made
relative to the _sync root_. Before #1694, they were made relative to
the _bundle root_. This PR didn't update the library glob expansion code
to use the sync root path.

If you were using the sync paths setting with library globs, the CLI
would fail to expand the globs because the code was using the wrong path
to anchor those globs.

This change fixes the issue.

## Tests

Manually confirmed that this fixes the issue reported in #1755.
This commit is contained in:
Pieter Noordhuis 2024-09-09 11:56:16 +02:00 committed by GitHub
parent 02e83877f4
commit b451905b6e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 49 additions and 52 deletions

View File

@ -39,7 +39,7 @@ func getLibDetails(v dyn.Value) (string, string, bool) {
} }
func findMatches(b *bundle.Bundle, path string) ([]string, error) { func findMatches(b *bundle.Bundle, path string) ([]string, error) {
matches, err := filepath.Glob(filepath.Join(b.RootPath, path)) matches, err := filepath.Glob(filepath.Join(b.SyncRootPath, path))
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -52,10 +52,10 @@ func findMatches(b *bundle.Bundle, path string) ([]string, error) {
} }
} }
// We make the matched path relative to the root path before storing it // We make the matched path relative to the sync root path before storing it
// to allow upload mutator to distinguish between local and remote paths // to allow upload mutator to distinguish between local and remote paths
for i, match := range matches { for i, match := range matches {
matches[i], err = filepath.Rel(b.RootPath, match) matches[i], err = filepath.Rel(b.SyncRootPath, match)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -211,8 +211,8 @@ func (e *expand) Name() string {
// ExpandGlobReferences expands any glob references in the libraries or environments section // ExpandGlobReferences expands any glob references in the libraries or environments section
// to corresponding local paths. // to corresponding local paths.
// We only expand local paths (i.e. paths that are relative to the root path). // We only expand local paths (i.e. paths that are relative to the sync root path).
// After expanding we make the paths relative to the root path to allow upload mutator later in the chain to // After expanding we make the paths relative to the sync root path to allow upload mutator later in the chain to
// distinguish between local and remote paths. // distinguish between local and remote paths.
func ExpandGlobReferences() bundle.Mutator { func ExpandGlobReferences() bundle.Mutator {
return &expand{} return &expand{}

View File

@ -23,7 +23,7 @@ func TestGlobReferencesExpandedForTaskLibraries(t *testing.T) {
testutil.Touch(t, dir, "jar", "my2.jar") testutil.Touch(t, dir, "jar", "my2.jar")
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, SyncRootPath: dir,
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
@ -104,7 +104,7 @@ func TestGlobReferencesExpandedForForeachTaskLibraries(t *testing.T) {
testutil.Touch(t, dir, "jar", "my2.jar") testutil.Touch(t, dir, "jar", "my2.jar")
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, SyncRootPath: dir,
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
@ -189,7 +189,7 @@ func TestGlobReferencesExpandedForEnvironmentsDeps(t *testing.T) {
testutil.Touch(t, dir, "jar", "my2.jar") testutil.Touch(t, dir, "jar", "my2.jar")
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, SyncRootPath: dir,
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{

View File

@ -18,7 +18,7 @@ func TestValidateEnvironments(t *testing.T) {
testutil.Touch(t, tmpDir, "wheel.whl") testutil.Touch(t, tmpDir, "wheel.whl")
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: tmpDir, SyncRootPath: tmpDir,
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
@ -50,7 +50,7 @@ func TestValidateEnvironmentsNoFile(t *testing.T) {
tmpDir := t.TempDir() tmpDir := t.TempDir()
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: tmpDir, SyncRootPath: tmpDir,
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
@ -84,7 +84,7 @@ func TestValidateTaskLibraries(t *testing.T) {
testutil.Touch(t, tmpDir, "wheel.whl") testutil.Touch(t, tmpDir, "wheel.whl")
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: tmpDir, SyncRootPath: tmpDir,
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
@ -117,7 +117,7 @@ func TestValidateTaskLibrariesNoFile(t *testing.T) {
tmpDir := t.TempDir() tmpDir := t.TempDir()
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: tmpDir, SyncRootPath: tmpDir,
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{

View File

@ -74,7 +74,7 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]configLocation, error
return v, nil return v, nil
} }
source = filepath.Join(b.RootPath, source) source = filepath.Join(b.SyncRootPath, source)
libs[source] = append(libs[source], configLocation{ libs[source] = append(libs[source], configLocation{
configPath: p, configPath: p,
location: v.Location(), location: v.Location(),

View File

@ -24,7 +24,7 @@ func TestArtifactUploadForWorkspace(t *testing.T) {
whlLocalPath := filepath.Join(whlFolder, "source.whl") whlLocalPath := filepath.Join(whlFolder, "source.whl")
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: tmpDir, SyncRootPath: tmpDir,
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
ArtifactPath: "/foo/bar/artifacts", ArtifactPath: "/foo/bar/artifacts",
@ -112,7 +112,7 @@ func TestArtifactUploadForVolumes(t *testing.T) {
whlLocalPath := filepath.Join(whlFolder, "source.whl") whlLocalPath := filepath.Join(whlFolder, "source.whl")
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: tmpDir, SyncRootPath: tmpDir,
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
ArtifactPath: "/Volumes/foo/bar/artifacts", ArtifactPath: "/Volumes/foo/bar/artifacts",
@ -200,7 +200,7 @@ func TestArtifactUploadWithNoLibraryReference(t *testing.T) {
whlLocalPath := filepath.Join(whlFolder, "source.whl") whlLocalPath := filepath.Join(whlFolder, "source.whl")
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: tmpDir, SyncRootPath: tmpDir,
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
ArtifactPath: "/Workspace/foo/bar/artifacts", ArtifactPath: "/Workspace/foo/bar/artifacts",
@ -240,7 +240,7 @@ func TestUploadMultipleLibraries(t *testing.T) {
testutil.Touch(t, whlFolder, "source4.whl") testutil.Touch(t, whlFolder, "source4.whl")
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: tmpDir, SyncRootPath: tmpDir,
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
ArtifactPath: "/foo/bar/artifacts", ArtifactPath: "/foo/bar/artifacts",

View File

@ -1,6 +1,9 @@
bundle: bundle:
name: python-wheel-local name: python-wheel-local
workspace:
artifact_path: /foo/bar
resources: resources:
jobs: jobs:
test_job: test_job:

View File

@ -15,11 +15,10 @@ import (
) )
func TestPythonWheelBuild(t *testing.T) { func TestPythonWheelBuild(t *testing.T) {
ctx := context.Background() b := loadTarget(t, "./python_wheel/python_wheel", "default")
b, err := bundle.Load(ctx, "./python_wheel/python_wheel")
require.NoError(t, err)
diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) ctx := context.Background()
diags := bundle.Apply(ctx, b, phases.Build())
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
matches, err := filepath.Glob("./python_wheel/python_wheel/my_test_code/dist/my_test_code-*.whl") matches, err := filepath.Glob("./python_wheel/python_wheel/my_test_code/dist/my_test_code-*.whl")
@ -32,11 +31,10 @@ func TestPythonWheelBuild(t *testing.T) {
} }
func TestPythonWheelBuildAutoDetect(t *testing.T) { func TestPythonWheelBuildAutoDetect(t *testing.T) {
ctx := context.Background() b := loadTarget(t, "./python_wheel/python_wheel_no_artifact", "default")
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_artifact")
require.NoError(t, err)
diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) ctx := context.Background()
diags := bundle.Apply(ctx, b, phases.Build())
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
matches, err := filepath.Glob("./python_wheel/python_wheel_no_artifact/dist/my_test_code-*.whl") matches, err := filepath.Glob("./python_wheel/python_wheel_no_artifact/dist/my_test_code-*.whl")
@ -49,11 +47,10 @@ func TestPythonWheelBuildAutoDetect(t *testing.T) {
} }
func TestPythonWheelBuildAutoDetectWithNotebookTask(t *testing.T) { func TestPythonWheelBuildAutoDetectWithNotebookTask(t *testing.T) {
ctx := context.Background() b := loadTarget(t, "./python_wheel/python_wheel_no_artifact_notebook", "default")
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_artifact_notebook")
require.NoError(t, err)
diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) ctx := context.Background()
diags := bundle.Apply(ctx, b, phases.Build())
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
matches, err := filepath.Glob("./python_wheel/python_wheel_no_artifact_notebook/dist/my_test_code-*.whl") matches, err := filepath.Glob("./python_wheel/python_wheel_no_artifact_notebook/dist/my_test_code-*.whl")
@ -66,11 +63,10 @@ func TestPythonWheelBuildAutoDetectWithNotebookTask(t *testing.T) {
} }
func TestPythonWheelWithDBFSLib(t *testing.T) { func TestPythonWheelWithDBFSLib(t *testing.T) {
ctx := context.Background() b := loadTarget(t, "./python_wheel/python_wheel_dbfs_lib", "default")
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_dbfs_lib")
require.NoError(t, err)
diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) ctx := context.Background()
diags := bundle.Apply(ctx, b, phases.Build())
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
match := libraries.ExpandGlobReferences() match := libraries.ExpandGlobReferences()
@ -79,11 +75,11 @@ func TestPythonWheelWithDBFSLib(t *testing.T) {
} }
func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) { func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) {
ctx := context.Background() b := loadTarget(t, "./python_wheel/python_wheel_no_artifact_no_setup", "default")
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_artifact_no_setup")
require.NoError(t, err)
b.Config.Workspace.ArtifactPath = "/foo/bar" ctx := context.Background()
diags := bundle.Apply(ctx, b, phases.Build())
require.NoError(t, diags.Error())
mockFiler := mockfiler.NewMockFiler(t) mockFiler := mockfiler.NewMockFiler(t)
mockFiler.EXPECT().Write( mockFiler.EXPECT().Write(
@ -94,20 +90,20 @@ func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) {
filer.CreateParentDirectories, filer.CreateParentDirectories,
).Return(nil) ).Return(nil)
u := libraries.UploadWithClient(mockFiler) diags = bundle.Apply(ctx, b, bundle.Seq(
diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build(), libraries.ExpandGlobReferences(), u)) libraries.ExpandGlobReferences(),
libraries.UploadWithClient(mockFiler),
))
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
require.Empty(t, diags) require.Empty(t, diags)
require.Equal(t, "/Workspace/foo/bar/.internal/my_test_code-0.0.1-py3-none-any.whl", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[0].Libraries[0].Whl) require.Equal(t, "/Workspace/foo/bar/.internal/my_test_code-0.0.1-py3-none-any.whl", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[0].Libraries[0].Whl)
} }
func TestPythonWheelBuildWithEnvironmentKey(t *testing.T) { func TestPythonWheelBuildWithEnvironmentKey(t *testing.T) {
ctx := context.Background() b := loadTarget(t, "./python_wheel/environment_key", "default")
b, err := bundle.Load(ctx, "./python_wheel/environment_key")
require.NoError(t, err)
diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) ctx := context.Background()
diags := bundle.Apply(ctx, b, phases.Build())
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
matches, err := filepath.Glob("./python_wheel/environment_key/my_test_code/dist/my_test_code-*.whl") matches, err := filepath.Glob("./python_wheel/environment_key/my_test_code/dist/my_test_code-*.whl")
@ -120,11 +116,10 @@ func TestPythonWheelBuildWithEnvironmentKey(t *testing.T) {
} }
func TestPythonWheelBuildMultiple(t *testing.T) { func TestPythonWheelBuildMultiple(t *testing.T) {
ctx := context.Background() b := loadTarget(t, "./python_wheel/python_wheel_multiple", "default")
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_multiple")
require.NoError(t, err)
diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) ctx := context.Background()
diags := bundle.Apply(ctx, b, phases.Build())
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
matches, err := filepath.Glob("./python_wheel/python_wheel_multiple/my_test_code/dist/my_test_code*.whl") matches, err := filepath.Glob("./python_wheel/python_wheel_multiple/my_test_code/dist/my_test_code*.whl")
@ -137,11 +132,10 @@ func TestPythonWheelBuildMultiple(t *testing.T) {
} }
func TestPythonWheelNoBuild(t *testing.T) { func TestPythonWheelNoBuild(t *testing.T) {
ctx := context.Background() b := loadTarget(t, "./python_wheel/python_wheel_no_build", "default")
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_build")
require.NoError(t, err)
diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build())) ctx := context.Background()
diags := bundle.Apply(ctx, b, phases.Build())
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
match := libraries.ExpandGlobReferences() match := libraries.ExpandGlobReferences()