From 6fde60c876451ed50f82f565e9aebee7fa56c5b6 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 18 Jul 2023 12:33:16 +0200 Subject: [PATCH 01/13] Added support for artifacts building for bundles --- bundle/artifacts/artifacts.go | 3 +++ bundle/artifacts/upload.go | 2 +- bundle/phases/deploy.go | 1 + libs/helpers.go | 11 +++++++++++ python/env_test.go | 3 ++- python/wheel.go | 5 +++-- 6 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 libs/helpers.go diff --git a/bundle/artifacts/artifacts.go b/bundle/artifacts/artifacts.go index c541312173..8298652f6e 100644 --- a/bundle/artifacts/artifacts.go +++ b/bundle/artifacts/artifacts.go @@ -63,6 +63,9 @@ func (m *basicBuild) Apply(ctx context.Context, b *bundle.Bundle) error { cmdio.LogString(ctx, fmt.Sprintf("artifacts.Build(%s): Building...", m.name)) + dir := artifact.Path + defer libs.ChdirAndBack(dir) + out, err := artifact.Build(ctx) if err != nil { return fmt.Errorf("artifacts.Build(%s): %w, output: %s", m.name, err, out) diff --git a/bundle/artifacts/upload.go b/bundle/artifacts/upload.go index 990718aa46..910071b533 100644 --- a/bundle/artifacts/upload.go +++ b/bundle/artifacts/upload.go @@ -66,5 +66,5 @@ func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) error { return fmt.Errorf("unable to create directory for %s: %w", uploadPath, err) } - return nil + return bundle.Apply(ctx, b, getUploadMutator(artifact.Type, m.name)) } diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 8b53273c7e..3f02155ca2 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -19,6 +19,7 @@ func Deploy() bundle.Mutator { libraries.MatchWithArtifacts(), artifacts.CleanUp(), artifacts.UploadAll(), + libraries.AttachToResources(), terraform.Interpolate(), terraform.Write(), terraform.StatePull(), diff --git a/libs/helpers.go b/libs/helpers.go new file mode 100644 index 0000000000..e124142052 --- /dev/null +++ b/libs/helpers.go @@ -0,0 +1,11 @@ +package libs + +import "os" + +func ChdirAndBack(dir string) func() { + wd, _ := os.Getwd() + os.Chdir(dir) + return func() { + os.Chdir(wd) + } +} diff --git a/python/env_test.go b/python/env_test.go index 5983ce38b0..a12f2a694f 100644 --- a/python/env_test.go +++ b/python/env_test.go @@ -5,6 +5,7 @@ import ( "runtime" "testing" + "github.com/databricks/cli/libs" "github.com/stretchr/testify/assert" ) @@ -31,7 +32,7 @@ func TestFreeze(t *testing.T) { } func TestPyInlineX(t *testing.T) { - defer chdirAndBack("testdata/simple-python-wheel")() + defer libs.ChdirAndBack("testdata/simple-python-wheel")() dist, err := ReadDistribution(context.Background()) assert.NoError(t, err) assert.Equal(t, "dummy", dist.Name) diff --git a/python/wheel.go b/python/wheel.go index 39c3d4cb4a..0814823e40 100644 --- a/python/wheel.go +++ b/python/wheel.go @@ -7,13 +7,14 @@ import ( "os" "path" + "github.com/databricks/cli/libs" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/files" ) func BuildWheel(ctx context.Context, dir string) (string, error) { - defer chdirAndBack(dir)() + defer libs.ChdirAndBack(dir)() // remove previous dist leak os.RemoveAll("dist") // remove all other irrelevant traces @@ -52,7 +53,7 @@ func UploadWheelToDBFSWithPEP503(ctx context.Context, dir string) (string, error if err != nil { return "", err } - defer chdirAndBack(dir)() + defer libs.ChdirAndBack(dir)() dist, err := ReadDistribution(ctx) if err != nil { return "", err From e2e2605a6b538694a0ce1e302145bdc89df07367 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 20 Jul 2023 13:48:23 +0200 Subject: [PATCH 02/13] dont use chdirAndBack --- bundle/artifacts/artifacts.go | 3 --- libs/helpers.go | 11 ----------- python/env_test.go | 3 +-- python/wheel.go | 5 ++--- 4 files changed, 3 insertions(+), 19 deletions(-) delete mode 100644 libs/helpers.go diff --git a/bundle/artifacts/artifacts.go b/bundle/artifacts/artifacts.go index 8298652f6e..c541312173 100644 --- a/bundle/artifacts/artifacts.go +++ b/bundle/artifacts/artifacts.go @@ -63,9 +63,6 @@ func (m *basicBuild) Apply(ctx context.Context, b *bundle.Bundle) error { cmdio.LogString(ctx, fmt.Sprintf("artifacts.Build(%s): Building...", m.name)) - dir := artifact.Path - defer libs.ChdirAndBack(dir) - out, err := artifact.Build(ctx) if err != nil { return fmt.Errorf("artifacts.Build(%s): %w, output: %s", m.name, err, out) diff --git a/libs/helpers.go b/libs/helpers.go deleted file mode 100644 index e124142052..0000000000 --- a/libs/helpers.go +++ /dev/null @@ -1,11 +0,0 @@ -package libs - -import "os" - -func ChdirAndBack(dir string) func() { - wd, _ := os.Getwd() - os.Chdir(dir) - return func() { - os.Chdir(wd) - } -} diff --git a/python/env_test.go b/python/env_test.go index a12f2a694f..5983ce38b0 100644 --- a/python/env_test.go +++ b/python/env_test.go @@ -5,7 +5,6 @@ import ( "runtime" "testing" - "github.com/databricks/cli/libs" "github.com/stretchr/testify/assert" ) @@ -32,7 +31,7 @@ func TestFreeze(t *testing.T) { } func TestPyInlineX(t *testing.T) { - defer libs.ChdirAndBack("testdata/simple-python-wheel")() + defer chdirAndBack("testdata/simple-python-wheel")() dist, err := ReadDistribution(context.Background()) assert.NoError(t, err) assert.Equal(t, "dummy", dist.Name) diff --git a/python/wheel.go b/python/wheel.go index 0814823e40..39c3d4cb4a 100644 --- a/python/wheel.go +++ b/python/wheel.go @@ -7,14 +7,13 @@ import ( "os" "path" - "github.com/databricks/cli/libs" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/files" ) func BuildWheel(ctx context.Context, dir string) (string, error) { - defer libs.ChdirAndBack(dir)() + defer chdirAndBack(dir)() // remove previous dist leak os.RemoveAll("dist") // remove all other irrelevant traces @@ -53,7 +52,7 @@ func UploadWheelToDBFSWithPEP503(ctx context.Context, dir string) (string, error if err != nil { return "", err } - defer libs.ChdirAndBack(dir)() + defer chdirAndBack(dir)() dist, err := ReadDistribution(ctx) if err != nil { return "", err From a4edb68b38f28265c810c5bee6998f6d6dd11bdd Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 20 Jul 2023 17:11:23 +0200 Subject: [PATCH 03/13] added test for build --- bundle/artifacts/build.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index 7721635a8e..df4ab1995a 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -51,5 +51,9 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { artifact.Path = filepath.Join(b.Config.Path, artifact.Path) } + if !filepath.IsAbs(artifact.Path) { + artifact.Path = filepath.Join(b.Config.Path, artifact.Path) + } + return bundle.Apply(ctx, b, getBuildMutator(artifact.Type, m.name)) } From c10043e46f309853f856d9a75306587aef463f1a Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 24 Jul 2023 12:34:08 +0200 Subject: [PATCH 04/13] explicit libraries block --- bundle/phases/deploy.go | 1 - 1 file changed, 1 deletion(-) diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 3f02155ca2..8b53273c7e 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -19,7 +19,6 @@ func Deploy() bundle.Mutator { libraries.MatchWithArtifacts(), artifacts.CleanUp(), artifacts.UploadAll(), - libraries.AttachToResources(), terraform.Interpolate(), terraform.Write(), terraform.StatePull(), From 141fcada2173a2951da5e6495fb2cf244f105e72 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 24 Jul 2023 17:50:45 +0200 Subject: [PATCH 05/13] artifact file can be used in multiple libraries --- bundle/artifacts/artifacts.go | 1 - 1 file changed, 1 deletion(-) diff --git a/bundle/artifacts/artifacts.go b/bundle/artifacts/artifacts.go index c541312173..c0b0cc9407 100644 --- a/bundle/artifacts/artifacts.go +++ b/bundle/artifacts/artifacts.go @@ -2,7 +2,6 @@ package artifacts import ( "context" - "crypto/sha256" "encoding/base64" "errors" "fmt" From 2510ffaa4be1e396dc1867d7b8c93d43652656b0 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 25 Jul 2023 10:39:11 +0200 Subject: [PATCH 06/13] addressed feedback --- bundle/artifacts/artifacts.go | 1 + 1 file changed, 1 insertion(+) diff --git a/bundle/artifacts/artifacts.go b/bundle/artifacts/artifacts.go index c0b0cc9407..c541312173 100644 --- a/bundle/artifacts/artifacts.go +++ b/bundle/artifacts/artifacts.go @@ -2,6 +2,7 @@ package artifacts import ( "context" + "crypto/sha256" "encoding/base64" "errors" "fmt" From da3735050a7e4c0a2e6ff1f517179191c80415b1 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 24 Jul 2023 18:52:46 +0200 Subject: [PATCH 07/13] Auto detect Python wheel packages and infer build command --- bundle/artifacts/autodetect.go | 30 +++++++++++++ bundle/artifacts/build.go | 4 -- bundle/artifacts/infer.go | 60 +++++++++++++++++++++++++ bundle/artifacts/whl/autodetect.go | 70 ++++++++++++++++++++++++++++++ bundle/artifacts/whl/build.go | 6 --- bundle/artifacts/whl/infer.go | 34 +++++++++++++++ bundle/phases/build.go | 2 + python/runner.go | 4 +- 8 files changed, 198 insertions(+), 12 deletions(-) create mode 100644 bundle/artifacts/autodetect.go create mode 100644 bundle/artifacts/infer.go create mode 100644 bundle/artifacts/whl/autodetect.go create mode 100644 bundle/artifacts/whl/infer.go diff --git a/bundle/artifacts/autodetect.go b/bundle/artifacts/autodetect.go new file mode 100644 index 0000000000..4644908184 --- /dev/null +++ b/bundle/artifacts/autodetect.go @@ -0,0 +1,30 @@ +package artifacts + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/artifacts/whl" +) + +func DetectPackages() bundle.Mutator { + return &autodetect{} +} + +type autodetect struct { +} + +func (m *autodetect) Name() string { + return "artifacts.DetectPackages" +} + +func (m *autodetect) Apply(ctx context.Context, b *bundle.Bundle) error { + // If artifacts section explicitly defined, do not try to auto detect packages + if b.Config.Artifacts != nil { + return nil + } + + return bundle.Apply(ctx, b, bundle.Seq( + whl.DetectPackage(), + )) +} diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index df4ab1995a..366d82db56 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -33,10 +33,6 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { return fmt.Errorf("artifact doesn't exist: %s", m.name) } - if len(artifact.Files) == 0 && artifact.BuildCommand == "" { - return fmt.Errorf("artifact %s misconfigured: 'files' or 'build' property is required", m.name) - } - // If artifact file is explicitly defined, skip building the artifact if len(artifact.Files) != 0 { return nil diff --git a/bundle/artifacts/infer.go b/bundle/artifacts/infer.go new file mode 100644 index 0000000000..a333e6998a --- /dev/null +++ b/bundle/artifacts/infer.go @@ -0,0 +1,60 @@ +package artifacts + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/artifacts/whl" + "github.com/databricks/cli/bundle/config" +) + +var inferMutators map[config.ArtifactType]mutatorFactory = map[config.ArtifactType]mutatorFactory{ + config.ArtifactPythonWheel: whl.InferBuildCommand, +} + +func getInferMutator(t config.ArtifactType, name string) bundle.Mutator { + mutatorFactory, ok := inferMutators[t] + if !ok { + return nil + } + + return mutatorFactory(name) +} + +func InferMissingProperties() bundle.Mutator { + return &all{ + name: "infer", + fn: inferArtifactByName, + } +} + +func inferArtifactByName(name string) (bundle.Mutator, error) { + return &infer{name}, nil +} + +type infer struct { + name string +} + +func (m *infer) Name() string { + return fmt.Sprintf("artifacts.Infer(%s)", m.name) +} + +func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) error { + artifact, ok := b.Config.Artifacts[m.name] + if !ok { + return fmt.Errorf("artifact doesn't exist: %s", m.name) + } + + if len(artifact.Files) > 0 || artifact.BuildCommand != "" { + return nil + } + + inferMutator := getInferMutator(artifact.Type, m.name) + if inferMutator != nil { + return bundle.Apply(ctx, b, inferMutator) + } + + return nil +} diff --git a/bundle/artifacts/whl/autodetect.go b/bundle/artifacts/whl/autodetect.go new file mode 100644 index 0000000000..24dc4f691a --- /dev/null +++ b/bundle/artifacts/whl/autodetect.go @@ -0,0 +1,70 @@ +package whl + +import ( + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "time" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/cmdio" +) + +type detectPkg struct { +} + +func DetectPackage() bundle.Mutator { + return &detectPkg{} +} + +func (m *detectPkg) Name() string { + return "artifacts.whl.AutoDetect" +} + +func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) error { + cmdio.LogString(ctx, "artifacts.whl.AutoDetect: Detecting Python wheel project...") + + // checking if there is setup.py in the bundle root + setupPy := filepath.Join(b.Config.Path, "setup.py") + _, err := os.Stat(setupPy) + if err != nil { + cmdio.LogString(ctx, "artifacts.whl.AutoDetect: No Python wheel project found at bundle root folder") + return nil + } + + cmdio.LogString(ctx, fmt.Sprintf("artifacts.whl.AutoDetect: Found Python wheel project at %s", b.Config.Path)) + module := extractModuleName(setupPy) + + if b.Config.Artifacts == nil { + b.Config.Artifacts = make(map[string]*config.Artifact) + } + + b.Config.Artifacts[module] = &config.Artifact{ + Path: b.Config.Path, + Type: config.ArtifactPythonWheel, + } + + return nil +} + +func extractModuleName(setupPy string) string { + bytes, err := os.ReadFile(setupPy) + if err != nil { + return randomName() + } + + content := string(bytes) + r := regexp.MustCompile("name=['\"](.*)['\"]") + matches := r.FindStringSubmatch(content) + if len(matches) == 0 { + return randomName() + } + return matches[1] +} + +func randomName() string { + return fmt.Sprintf("artifact%d", time.Now().Unix()) +} diff --git a/bundle/artifacts/whl/build.go b/bundle/artifacts/whl/build.go index 4ee47153b8..4565a4c80c 100644 --- a/bundle/artifacts/whl/build.go +++ b/bundle/artifacts/whl/build.go @@ -32,12 +32,6 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { return fmt.Errorf("artifact doesn't exist: %s", m.name) } - // TODO: If not set, BuildCommand should be infer prior to this - // via a mutator so that it can be observable. - if artifact.BuildCommand == "" { - return fmt.Errorf("artifacts.whl.Build(%s): missing build property for the artifact", m.name) - } - cmdio.LogString(ctx, fmt.Sprintf("artifacts.whl.Build(%s): Building...", m.name)) dir := artifact.Path diff --git a/bundle/artifacts/whl/infer.go b/bundle/artifacts/whl/infer.go new file mode 100644 index 0000000000..518d926ca5 --- /dev/null +++ b/bundle/artifacts/whl/infer.go @@ -0,0 +1,34 @@ +package whl + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/python" +) + +type infer struct { + name string +} + +func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) error { + artifact := b.Config.Artifacts[m.name] + py, err := python.DetectExecutable(ctx) + if err != nil { + return err + } + artifact.BuildCommand = fmt.Sprintf("%s setup.py bdist_wheel", py) + + return nil +} + +func (m *infer) Name() string { + return fmt.Sprintf("artifacts.whl.Infer(%s)", m.name) +} + +func InferBuildCommand(name string) bundle.Mutator { + return &infer{ + name: name, + } +} diff --git a/bundle/phases/build.go b/bundle/phases/build.go index 9249c32c0e..fe90c3691e 100644 --- a/bundle/phases/build.go +++ b/bundle/phases/build.go @@ -11,6 +11,8 @@ func Build() bundle.Mutator { return newPhase( "build", []bundle.Mutator{ + artifacts.DetectPackages(), + artifacts.InferMissingProperties(), artifacts.BuildAll(), interpolation.Interpolate( interpolation.IncludeLookupsInPath("artifacts"), diff --git a/python/runner.go b/python/runner.go index 6145da2774..b2946b2971 100644 --- a/python/runner.go +++ b/python/runner.go @@ -15,7 +15,7 @@ func PyInline(ctx context.Context, inlinePy string) (string, error) { } func Py(ctx context.Context, script string, args ...string) (string, error) { - py, err := detectExecutable(ctx) + py, err := DetectExecutable(ctx) if err != nil { return "", err } @@ -70,7 +70,7 @@ func detectVirtualEnv() (string, error) { var pyExec string -func detectExecutable(ctx context.Context) (string, error) { +func DetectExecutable(ctx context.Context) (string, error) { if pyExec != "" { return pyExec, nil } From 0eea175637ac201dddfb4745378fae8ad00698aa Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 24 Jul 2023 18:58:16 +0200 Subject: [PATCH 08/13] fixed tests --- python/runner_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/runner_test.go b/python/runner_test.go index 321a1b7dcf..b43d218ce1 100644 --- a/python/runner_test.go +++ b/python/runner_test.go @@ -25,14 +25,14 @@ func TestExecAndPassError(t *testing.T) { func TestDetectPython(t *testing.T) { pyExec = "" - py, err := detectExecutable(context.Background()) + py, err := DetectExecutable(context.Background()) assert.NoError(t, err) assert.Contains(t, py, "python3") } func TestDetectPythonCache(t *testing.T) { pyExec = "abc" - py, err := detectExecutable(context.Background()) + py, err := DetectExecutable(context.Background()) assert.NoError(t, err) assert.Equal(t, "abc", py) pyExec = "" @@ -82,7 +82,7 @@ func TestPyInline(t *testing.T) { } func TestPyInlineStderr(t *testing.T) { - detectExecutable(context.Background()) + DetectExecutable(context.Background()) inline := "import sys; sys.stderr.write('___msg___'); sys.exit(1)" _, err := PyInline(context.Background(), inline) assert.EqualError(t, err, "___msg___") From 55f2e2e119aea00b34672095e45d2e3402ce4fdc Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 25 Jul 2023 10:46:36 +0200 Subject: [PATCH 09/13] minor fixes --- bundle/artifacts/autodetect.go | 2 ++ bundle/artifacts/whl/autodetect.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bundle/artifacts/autodetect.go b/bundle/artifacts/autodetect.go index 4644908184..fa8126f973 100644 --- a/bundle/artifacts/autodetect.go +++ b/bundle/artifacts/autodetect.go @@ -5,6 +5,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/artifacts/whl" + "github.com/databricks/cli/libs/log" ) func DetectPackages() bundle.Mutator { @@ -21,6 +22,7 @@ func (m *autodetect) Name() string { func (m *autodetect) Apply(ctx context.Context, b *bundle.Bundle) error { // If artifacts section explicitly defined, do not try to auto detect packages if b.Config.Artifacts != nil { + log.Debugf(ctx, "artifacts block is defined, skipping auto-detecting") return nil } diff --git a/bundle/artifacts/whl/autodetect.go b/bundle/artifacts/whl/autodetect.go index 24dc4f691a..863de1840f 100644 --- a/bundle/artifacts/whl/autodetect.go +++ b/bundle/artifacts/whl/autodetect.go @@ -57,7 +57,7 @@ func extractModuleName(setupPy string) string { } content := string(bytes) - r := regexp.MustCompile("name=['\"](.*)['\"]") + r := regexp.MustCompile(`name=['"](.*)['"]`) matches := r.FindStringSubmatch(content) if len(matches) == 0 { return randomName() From ebfcba00e2abb6b6f8b6c264059546d0ecbd4bca Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 25 Jul 2023 13:56:03 +0200 Subject: [PATCH 10/13] rebase --- bundle/artifacts/upload.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/artifacts/upload.go b/bundle/artifacts/upload.go index 910071b533..990718aa46 100644 --- a/bundle/artifacts/upload.go +++ b/bundle/artifacts/upload.go @@ -66,5 +66,5 @@ func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) error { return fmt.Errorf("unable to create directory for %s: %w", uploadPath, err) } - return bundle.Apply(ctx, b, getUploadMutator(artifact.Type, m.name)) + return nil } From 8bbbf84b0b16f5b831b5c2a4c88956dd0c5bcdf7 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 25 Jul 2023 13:58:51 +0200 Subject: [PATCH 11/13] typo --- bundle/artifacts/build.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index 366d82db56..f3a7965a98 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -47,9 +47,5 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { artifact.Path = filepath.Join(b.Config.Path, artifact.Path) } - if !filepath.IsAbs(artifact.Path) { - artifact.Path = filepath.Join(b.Config.Path, artifact.Path) - } - return bundle.Apply(ctx, b, getBuildMutator(artifact.Type, m.name)) } From 860e8c484775de007c9924173b7eee62fcf94232 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 26 Jul 2023 11:21:15 +0200 Subject: [PATCH 12/13] added tests --- bundle/artifacts/build.go | 9 ++++++-- bundle/artifacts/infer.go | 2 +- bundle/artifacts/whl/autodetect.go | 6 ++++- bundle/artifacts/whl/autodetect_test.go | 22 +++++++++++++++++++ bundle/artifacts/whl/testdata/setup.py | 15 +++++++++++++ .../artifacts/whl/testdata/setup_incorrect.py | 14 ++++++++++++ .../artifacts/whl/testdata/setup_minimal.py | 3 +++ .../python_wheel_no_artifact/.gitignore | 3 +++ .../python_wheel_no_artifact/bundle.yml | 13 +++++++++++ .../my_test_code/__init__.py | 2 ++ .../my_test_code/__main__.py | 16 ++++++++++++++ .../bundle/python_wheel_no_artifact/setup.py | 15 +++++++++++++ bundle/tests/bundle/wheel_test.go | 13 +++++++++++ 13 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 bundle/artifacts/whl/autodetect_test.go create mode 100644 bundle/artifacts/whl/testdata/setup.py create mode 100644 bundle/artifacts/whl/testdata/setup_incorrect.py create mode 100644 bundle/artifacts/whl/testdata/setup_minimal.py create mode 100644 bundle/tests/bundle/python_wheel_no_artifact/.gitignore create mode 100644 bundle/tests/bundle/python_wheel_no_artifact/bundle.yml create mode 100644 bundle/tests/bundle/python_wheel_no_artifact/my_test_code/__init__.py create mode 100644 bundle/tests/bundle/python_wheel_no_artifact/my_test_code/__main__.py create mode 100644 bundle/tests/bundle/python_wheel_no_artifact/setup.py diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index f3a7965a98..6b1aac8226 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -33,8 +33,13 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { return fmt.Errorf("artifact doesn't exist: %s", m.name) } - // If artifact file is explicitly defined, skip building the artifact - if len(artifact.Files) != 0 { + // Skip building if build command is not specified or infered + if artifact.BuildCommand == "" { + // If no build command was specified or infered and there is no + // artifact output files specified, artifact is misconfigured + if len(artifact.Files) == 0 { + return fmt.Errorf("misconfigured artifact: please specify 'build' or 'files' property") + } return nil } diff --git a/bundle/artifacts/infer.go b/bundle/artifacts/infer.go index a333e6998a..233fbda86b 100644 --- a/bundle/artifacts/infer.go +++ b/bundle/artifacts/infer.go @@ -47,7 +47,7 @@ func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) error { return fmt.Errorf("artifact doesn't exist: %s", m.name) } - if len(artifact.Files) > 0 || artifact.BuildCommand != "" { + if artifact.BuildCommand != "" { return nil } diff --git a/bundle/artifacts/whl/autodetect.go b/bundle/artifacts/whl/autodetect.go index 863de1840f..a801b48d75 100644 --- a/bundle/artifacts/whl/autodetect.go +++ b/bundle/artifacts/whl/autodetect.go @@ -42,8 +42,12 @@ func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) error { b.Config.Artifacts = make(map[string]*config.Artifact) } + pkgPath, err := filepath.Abs(b.Config.Path) + if err != nil { + return err + } b.Config.Artifacts[module] = &config.Artifact{ - Path: b.Config.Path, + Path: pkgPath, Type: config.ArtifactPythonWheel, } diff --git a/bundle/artifacts/whl/autodetect_test.go b/bundle/artifacts/whl/autodetect_test.go new file mode 100644 index 0000000000..b53289b2ad --- /dev/null +++ b/bundle/artifacts/whl/autodetect_test.go @@ -0,0 +1,22 @@ +package whl + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestExtractModuleName(t *testing.T) { + moduleName := extractModuleName("./testdata/setup.py") + assert.Equal(t, "my_test_code", moduleName) +} + +func TestExtractModuleNameMinimal(t *testing.T) { + moduleName := extractModuleName("./testdata/setup_minimal.py") + assert.Equal(t, "my_test_code", moduleName) +} + +func TestExtractModuleNameIncorrect(t *testing.T) { + moduleName := extractModuleName("./testdata/setup_incorrect.py") + assert.Contains(t, moduleName, "artifact") +} diff --git a/bundle/artifacts/whl/testdata/setup.py b/bundle/artifacts/whl/testdata/setup.py new file mode 100644 index 0000000000..7a1317b2f5 --- /dev/null +++ b/bundle/artifacts/whl/testdata/setup.py @@ -0,0 +1,15 @@ +from setuptools import setup, find_packages + +import my_test_code + +setup( + name="my_test_code", + version=my_test_code.__version__, + author=my_test_code.__author__, + url="https://databricks.com", + author_email="john.doe@databricks.com", + description="my test wheel", + packages=find_packages(include=["my_test_code"]), + entry_points={"group_1": "run=my_test_code.__main__:main"}, + install_requires=["setuptools"], +) diff --git a/bundle/artifacts/whl/testdata/setup_incorrect.py b/bundle/artifacts/whl/testdata/setup_incorrect.py new file mode 100644 index 0000000000..c6aa17b2d8 --- /dev/null +++ b/bundle/artifacts/whl/testdata/setup_incorrect.py @@ -0,0 +1,14 @@ +from setuptools import setup, find_packages + +import my_test_code + +setup( + version=my_test_code.__version__, + author=my_test_code.__author__, + url="https://databricks.com", + author_email="john.doe@databricks.com", + description="my test wheel", + packages=find_packages(include=["my_test_code"]), + entry_points={"group_1": "run=my_test_code.__main__:main"}, + install_requires=["setuptools"], +) diff --git a/bundle/artifacts/whl/testdata/setup_minimal.py b/bundle/artifacts/whl/testdata/setup_minimal.py new file mode 100644 index 0000000000..3e81e72173 --- /dev/null +++ b/bundle/artifacts/whl/testdata/setup_minimal.py @@ -0,0 +1,3 @@ +from setuptools import setup + +setup(name="my_test_code") diff --git a/bundle/tests/bundle/python_wheel_no_artifact/.gitignore b/bundle/tests/bundle/python_wheel_no_artifact/.gitignore new file mode 100644 index 0000000000..f03e23bc26 --- /dev/null +++ b/bundle/tests/bundle/python_wheel_no_artifact/.gitignore @@ -0,0 +1,3 @@ +build/ +*.egg-info +.databricks diff --git a/bundle/tests/bundle/python_wheel_no_artifact/bundle.yml b/bundle/tests/bundle/python_wheel_no_artifact/bundle.yml new file mode 100644 index 0000000000..1090867295 --- /dev/null +++ b/bundle/tests/bundle/python_wheel_no_artifact/bundle.yml @@ -0,0 +1,13 @@ +bundle: + name: python-wheel + +resources: + jobs: + test_job: + name: "[${bundle.environment}] My Wheel Job" + tasks: + - task_key: TestTask + existing_cluster_id: "0717-aaaaa-bbbbbb" + python_wheel_task: + package_name: "my_test_code" + entry_point: "run" diff --git a/bundle/tests/bundle/python_wheel_no_artifact/my_test_code/__init__.py b/bundle/tests/bundle/python_wheel_no_artifact/my_test_code/__init__.py new file mode 100644 index 0000000000..909f1f3220 --- /dev/null +++ b/bundle/tests/bundle/python_wheel_no_artifact/my_test_code/__init__.py @@ -0,0 +1,2 @@ +__version__ = "0.0.1" +__author__ = "Databricks" diff --git a/bundle/tests/bundle/python_wheel_no_artifact/my_test_code/__main__.py b/bundle/tests/bundle/python_wheel_no_artifact/my_test_code/__main__.py new file mode 100644 index 0000000000..73d045afb4 --- /dev/null +++ b/bundle/tests/bundle/python_wheel_no_artifact/my_test_code/__main__.py @@ -0,0 +1,16 @@ +""" +The entry point of the Python Wheel +""" + +import sys + + +def main(): + # This method will print the provided arguments + print('Hello from my func') + print('Got arguments:') + print(sys.argv) + + +if __name__ == '__main__': + main() diff --git a/bundle/tests/bundle/python_wheel_no_artifact/setup.py b/bundle/tests/bundle/python_wheel_no_artifact/setup.py new file mode 100644 index 0000000000..7a1317b2f5 --- /dev/null +++ b/bundle/tests/bundle/python_wheel_no_artifact/setup.py @@ -0,0 +1,15 @@ +from setuptools import setup, find_packages + +import my_test_code + +setup( + name="my_test_code", + version=my_test_code.__version__, + author=my_test_code.__author__, + url="https://databricks.com", + author_email="john.doe@databricks.com", + description="my test wheel", + packages=find_packages(include=["my_test_code"]), + entry_points={"group_1": "run=my_test_code.__main__:main"}, + install_requires=["setuptools"], +) diff --git a/bundle/tests/bundle/wheel_test.go b/bundle/tests/bundle/wheel_test.go index 5b786185b4..2290e47c68 100644 --- a/bundle/tests/bundle/wheel_test.go +++ b/bundle/tests/bundle/wheel_test.go @@ -22,3 +22,16 @@ func TestBundlePythonWheelBuild(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, len(matches)) } + +func TestBundlePythonWheelBuildAutoDetect(t *testing.T) { + b, err := bundle.Load("./python_wheel_no_artifact") + require.NoError(t, err) + + m := phases.Build() + err = m.Apply(context.Background(), b) + require.NoError(t, err) + + matches, err := filepath.Glob("python_wheel/my_test_code/dist/my_test_code-*.whl") + require.NoError(t, err) + require.Equal(t, 1, len(matches)) +} From 7396101fee2f0b51423b7f52c2c0f1e7aed7d59d Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 26 Jul 2023 11:46:31 +0200 Subject: [PATCH 13/13] use python3 --- .github/workflows/push.yml | 2 +- bundle/tests/bundle/python_wheel/bundle.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 2406ed71f2..0f9b601616 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -41,7 +41,7 @@ jobs: - name: Pull external libraries run: | make vendor - pip install wheel + pip3 install wheel - name: Run tests run: make test diff --git a/bundle/tests/bundle/python_wheel/bundle.yml b/bundle/tests/bundle/python_wheel/bundle.yml index b3a793a63d..4e272c9f5a 100644 --- a/bundle/tests/bundle/python_wheel/bundle.yml +++ b/bundle/tests/bundle/python_wheel/bundle.yml @@ -5,7 +5,7 @@ artifacts: my_test_code: type: whl path: "./my_test_code" - build: "python setup.py bdist_wheel" + build: "python3 setup.py bdist_wheel" resources: jobs: