Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand library globs relative to the sync root #1756

Merged
merged 2 commits into from
Sep 9, 2024
Merged

Expand library globs relative to the sync root #1756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f42d8b
Expand library globs relative to the sync root
pietern Sep 9, 2024
ec56a14
Use test helpers in python wheel tests to ensure the sync root path i…
pietern Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions bundle/libraries/expand_glob_references.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func getLibDetails(v dyn.Value) (string, string, bool) {
}

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 {
return nil, err
}
Expand All @@ -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
for i, match := range matches {
matches[i], err = filepath.Rel(b.RootPath, match)
matches[i], err = filepath.Rel(b.SyncRootPath, match)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -211,8 +211,8 @@ func (e *expand) Name() string {

// ExpandGlobReferences expands any glob references in the libraries or environments section
// to corresponding local paths.
// We only expand local paths (i.e. paths that are relative to the root path).
// After expanding we make the paths relative to the root path to allow upload mutator later in the chain to
// 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 sync root path to allow upload mutator later in the chain to
// distinguish between local and remote paths.
func ExpandGlobReferences() bundle.Mutator {
return &expand{}
Expand Down
6 changes: 3 additions & 3 deletions bundle/libraries/expand_glob_references_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestGlobReferencesExpandedForTaskLibraries(t *testing.T) {
testutil.Touch(t, dir, "jar", "my2.jar")

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

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

b := &bundle.Bundle{
RootPath: dir,
SyncRootPath: dir,
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
Expand Down
8 changes: 4 additions & 4 deletions bundle/libraries/match_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestValidateEnvironments(t *testing.T) {
testutil.Touch(t, tmpDir, "wheel.whl")

b := &bundle.Bundle{
RootPath: tmpDir,
SyncRootPath: tmpDir,
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
Expand Down Expand Up @@ -50,7 +50,7 @@ func TestValidateEnvironmentsNoFile(t *testing.T) {
tmpDir := t.TempDir()

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

b := &bundle.Bundle{
RootPath: tmpDir,
SyncRootPath: tmpDir,
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
Expand Down Expand Up @@ -117,7 +117,7 @@ func TestValidateTaskLibrariesNoFile(t *testing.T) {
tmpDir := t.TempDir()

b := &bundle.Bundle{
RootPath: tmpDir,
SyncRootPath: tmpDir,
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
Expand Down
2 changes: 1 addition & 1 deletion bundle/libraries/upload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]configLocation, error
return v, nil
}

source = filepath.Join(b.RootPath, source)
source = filepath.Join(b.SyncRootPath, source)
libs[source] = append(libs[source], configLocation{
configPath: p,
location: v.Location(),
Expand Down
8 changes: 4 additions & 4 deletions bundle/libraries/upload_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestArtifactUploadForWorkspace(t *testing.T) {
whlLocalPath := filepath.Join(whlFolder, "source.whl")

b := &bundle.Bundle{
RootPath: tmpDir,
SyncRootPath: tmpDir,
Config: config.Root{
Workspace: config.Workspace{
ArtifactPath: "/foo/bar/artifacts",
Expand Down Expand Up @@ -112,7 +112,7 @@ func TestArtifactUploadForVolumes(t *testing.T) {
whlLocalPath := filepath.Join(whlFolder, "source.whl")

b := &bundle.Bundle{
RootPath: tmpDir,
SyncRootPath: tmpDir,
Config: config.Root{
Workspace: config.Workspace{
ArtifactPath: "/Volumes/foo/bar/artifacts",
Expand Down Expand Up @@ -200,7 +200,7 @@ func TestArtifactUploadWithNoLibraryReference(t *testing.T) {
whlLocalPath := filepath.Join(whlFolder, "source.whl")

b := &bundle.Bundle{
RootPath: tmpDir,
SyncRootPath: tmpDir,
Config: config.Root{
Workspace: config.Workspace{
ArtifactPath: "/Workspace/foo/bar/artifacts",
Expand Down Expand Up @@ -240,7 +240,7 @@ func TestUploadMultipleLibraries(t *testing.T) {
testutil.Touch(t, whlFolder, "source4.whl")

b := &bundle.Bundle{
RootPath: tmpDir,
SyncRootPath: tmpDir,
Config: config.Root{
Workspace: config.Workspace{
ArtifactPath: "/foo/bar/artifacts",
Expand Down
3 changes: 3 additions & 0 deletions bundle/tests/python_wheel/python_wheel_no_artifact_no_setup/bundle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
bundle:
name: python-wheel-local

workspace:
artifact_path: /foo/bar

resources:
jobs:
test_job:
Expand Down
64 changes: 29 additions & 35 deletions bundle/tests/python_wheel_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ import (
)

func TestPythonWheelBuild(t *testing.T) {
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel/python_wheel")
require.NoError(t, err)
b := loadTarget(t, "./python_wheel/python_wheel", "default")

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())

matches, err := filepath.Glob("./python_wheel/python_wheel/my_test_code/dist/my_test_code-*.whl")
Expand All @@ -32,11 +31,10 @@ func TestPythonWheelBuild(t *testing.T) {
}

func TestPythonWheelBuildAutoDetect(t *testing.T) {
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_artifact")
require.NoError(t, err)
b := loadTarget(t, "./python_wheel/python_wheel_no_artifact", "default")

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())

matches, err := filepath.Glob("./python_wheel/python_wheel_no_artifact/dist/my_test_code-*.whl")
Expand All @@ -49,11 +47,10 @@ func TestPythonWheelBuildAutoDetect(t *testing.T) {
}

func TestPythonWheelBuildAutoDetectWithNotebookTask(t *testing.T) {
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_artifact_notebook")
require.NoError(t, err)
b := loadTarget(t, "./python_wheel/python_wheel_no_artifact_notebook", "default")

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())

matches, err := filepath.Glob("./python_wheel/python_wheel_no_artifact_notebook/dist/my_test_code-*.whl")
Expand All @@ -66,11 +63,10 @@ func TestPythonWheelBuildAutoDetectWithNotebookTask(t *testing.T) {
}

func TestPythonWheelWithDBFSLib(t *testing.T) {
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_dbfs_lib")
require.NoError(t, err)
b := loadTarget(t, "./python_wheel/python_wheel_dbfs_lib", "default")

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())

match := libraries.ExpandGlobReferences()
Expand All @@ -79,11 +75,11 @@ func TestPythonWheelWithDBFSLib(t *testing.T) {
}

func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) {
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_artifact_no_setup")
require.NoError(t, err)
b := loadTarget(t, "./python_wheel/python_wheel_no_artifact_no_setup", "default")

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.EXPECT().Write(
Expand All @@ -94,20 +90,20 @@ func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) {
filer.CreateParentDirectories,
).Return(nil)

u := libraries.UploadWithClient(mockFiler)
diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build(), libraries.ExpandGlobReferences(), u))
diags = bundle.Apply(ctx, b, bundle.Seq(
libraries.ExpandGlobReferences(),
libraries.UploadWithClient(mockFiler),
))
require.NoError(t, diags.Error())
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)
}

func TestPythonWheelBuildWithEnvironmentKey(t *testing.T) {
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel/environment_key")
require.NoError(t, err)
b := loadTarget(t, "./python_wheel/environment_key", "default")

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())

matches, err := filepath.Glob("./python_wheel/environment_key/my_test_code/dist/my_test_code-*.whl")
Expand All @@ -120,11 +116,10 @@ func TestPythonWheelBuildWithEnvironmentKey(t *testing.T) {
}

func TestPythonWheelBuildMultiple(t *testing.T) {
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_multiple")
require.NoError(t, err)
b := loadTarget(t, "./python_wheel/python_wheel_multiple", "default")

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())

matches, err := filepath.Glob("./python_wheel/python_wheel_multiple/my_test_code/dist/my_test_code*.whl")
Expand All @@ -137,11 +132,10 @@ func TestPythonWheelBuildMultiple(t *testing.T) {
}

func TestPythonWheelNoBuild(t *testing.T) {
ctx := context.Background()
b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_build")
require.NoError(t, err)
b := loadTarget(t, "./python_wheel/python_wheel_no_build", "default")

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())

match := libraries.ExpandGlobReferences()
Expand Down
Loading