Skip to content

Commit

Permalink
Make pydabs/venv_path optional (#1687)
Browse files Browse the repository at this point in the history
## Changes
Make `pydabs/venv_path` optional. When not specified, CLI detects the
Python interpreter using `python.DetectExecutable`, the same way as for
`artifacts`. `python.DetectExecutable` works correctly if a virtual
environment is activated or `python3` is available on PATH through other
means.

Extract the venv detection code from PyDABs into `libs/python/detect`.
This code will be used when we implement the `python/venv_path` section
in `databricks.yml`.

## Tests
Unit tests and manually

---------

Co-authored-by: Pieter Noordhuis <[email protected]>
  • Loading branch information
kanterov and pietern authored Aug 20, 2024
1 parent af5048e commit 44902fa
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 24 deletions.
2 changes: 2 additions & 0 deletions bundle/artifacts/whl/infer.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ type infer struct {

func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
artifact := b.Config.Artifacts[m.name]

// TODO use python.DetectVEnvExecutable once bundle has a way to specify venv path
py, err := python.DetectExecutable(ctx)
if err != nil {
return diag.FromErr(err)
Expand Down
4 changes: 2 additions & 2 deletions bundle/config/experimental.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ type PyDABs struct {

// VEnvPath is path to the virtual environment.
//
// Required if PyDABs is enabled. PyDABs will load the code in the specified
// environment.
// If enabled, PyDABs will execute code within this environment. If disabled,
// it defaults to using the Python interpreter available in the current shell.
VEnvPath string `json:"venv_path,omitempty"`

// Import contains a list Python packages with PyDABs code.
Expand Down
33 changes: 15 additions & 18 deletions bundle/config/mutator/python/python_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"fmt"
"os"
"path/filepath"
"runtime"

"github.com/databricks/cli/libs/python"
"github.com/databricks/databricks-sdk-go/logger"

"github.com/databricks/cli/bundle/env"
Expand Down Expand Up @@ -86,23 +86,15 @@ func (m *pythonMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagno
return nil
}

if experimental.PyDABs.VEnvPath == "" {
return diag.Errorf("\"experimental.pydabs.enabled\" can only be used when \"experimental.pydabs.venv_path\" is set")
}

// mutateDiags is used because Mutate returns 'error' instead of 'diag.Diagnostics'
var mutateDiags diag.Diagnostics
var mutateDiagsHasError = errors.New("unexpected error")

err := b.Config.Mutate(func(leftRoot dyn.Value) (dyn.Value, error) {
pythonPath := interpreterPath(experimental.PyDABs.VEnvPath)
pythonPath, err := detectExecutable(ctx, experimental.PyDABs.VEnvPath)

if _, err := os.Stat(pythonPath); err != nil {
if os.IsNotExist(err) {
return dyn.InvalidValue, fmt.Errorf("can't find %q, check if venv is created", pythonPath)
} else {
return dyn.InvalidValue, fmt.Errorf("can't find %q: %w", pythonPath, err)
}
if err != nil {
return dyn.InvalidValue, fmt.Errorf("failed to get Python interpreter path: %w", err)
}

cacheDir, err := createCacheDir(ctx)
Expand Down Expand Up @@ -423,11 +415,16 @@ func isOmitemptyDelete(left dyn.Value) bool {
}
}

// interpreterPath returns platform-specific path to Python interpreter in the virtual environment.
func interpreterPath(venvPath string) string {
if runtime.GOOS == "windows" {
return filepath.Join(venvPath, "Scripts", "python3.exe")
} else {
return filepath.Join(venvPath, "bin", "python3")
// detectExecutable lookups Python interpreter in virtual environment, or if not set, in PATH.
func detectExecutable(ctx context.Context, venvPath string) (string, error) {
if venvPath == "" {
interpreter, err := python.DetectExecutable(ctx)
if err != nil {
return "", err
}

return interpreter, nil
}

return python.DetectVEnvExecutable(venvPath)
}
21 changes: 17 additions & 4 deletions bundle/config/mutator/python/python_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ func TestPythonMutator_venvRequired(t *testing.T) {
}

func TestPythonMutator_venvNotFound(t *testing.T) {
expectedError := fmt.Sprintf("can't find %q, check if venv is created", interpreterPath("bad_path"))
expectedError := fmt.Sprintf("failed to get Python interpreter path: can't find %q, check if virtualenv is created", interpreterPath("bad_path"))

b := loadYaml("databricks.yml", `
experimental:
Expand Down Expand Up @@ -596,9 +596,7 @@ func loadYaml(name string, content string) *bundle.Bundle {
}
}

func withFakeVEnv(t *testing.T, path string) {
interpreterPath := interpreterPath(path)

func withFakeVEnv(t *testing.T, venvPath string) {
cwd, err := os.Getwd()
if err != nil {
panic(err)
Expand All @@ -608,6 +606,8 @@ func withFakeVEnv(t *testing.T, path string) {
panic(err)
}

interpreterPath := interpreterPath(venvPath)

err = os.MkdirAll(filepath.Dir(interpreterPath), 0755)
if err != nil {
panic(err)
Expand All @@ -618,9 +618,22 @@ func withFakeVEnv(t *testing.T, path string) {
panic(err)
}

err = os.WriteFile(filepath.Join(venvPath, "pyvenv.cfg"), []byte(""), 0755)
if err != nil {
panic(err)
}

t.Cleanup(func() {
if err := os.Chdir(cwd); err != nil {
panic(err)
}
})
}

func interpreterPath(venvPath string) string {
if runtime.GOOS == "windows" {
return filepath.Join(venvPath, "Scripts", "python3.exe")
} else {
return filepath.Join(venvPath, "bin", "python3")
}
}
46 changes: 46 additions & 0 deletions libs/python/detect.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,23 @@ package python
import (
"context"
"errors"
"fmt"
"io/fs"
"os"
"os/exec"
"path/filepath"
"runtime"
)

// DetectExecutable looks up the path to the python3 executable from the PATH
// environment variable.
//
// If virtualenv is activated, executable from the virtualenv is returned,
// because activating virtualenv adds python3 executable on a PATH.
//
// If python3 executable is not found on the PATH, the interpreter with the
// least version that satisfies minimal 3.8 version is returned, e.g.
// python3.10.
func DetectExecutable(ctx context.Context) (string, error) {
// TODO: add a shortcut if .python-version file is detected somewhere in
// the parent directory tree.
Expand All @@ -32,3 +46,35 @@ func DetectExecutable(ctx context.Context) (string, error) {
}
return interpreter.Path, nil
}

// DetectVEnvExecutable returns the path to the python3 executable inside venvPath,
// that is not necessarily activated.
//
// If virtualenv is not created, or executable doesn't exist, the error is returned.
func DetectVEnvExecutable(venvPath string) (string, error) {
interpreterPath := filepath.Join(venvPath, "bin", "python3")
if runtime.GOOS == "windows" {
interpreterPath = filepath.Join(venvPath, "Scripts", "python3.exe")
}

if _, err := os.Stat(interpreterPath); err != nil {
if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("can't find %q, check if virtualenv is created", interpreterPath)
} else {
return "", fmt.Errorf("can't find %q: %w", interpreterPath, err)
}
}

// pyvenv.cfg must be always present in correctly configured virtualenv,
// read more in https://snarky.ca/how-virtual-environments-work/
pyvenvPath := filepath.Join(venvPath, "pyvenv.cfg")
if _, err := os.Stat(pyvenvPath); err != nil {
if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("expected %q to be virtualenv, but pyvenv.cfg is missing", venvPath)
} else {
return "", fmt.Errorf("can't find %q: %w", pyvenvPath, err)
}
}

return interpreterPath, nil
}
46 changes: 46 additions & 0 deletions libs/python/detect_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package python

import (
"os"
"path/filepath"
"runtime"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestDetectVEnvExecutable(t *testing.T) {
dir := t.TempDir()
interpreterPath := interpreterPath(dir)

err := os.Mkdir(filepath.Dir(interpreterPath), 0755)
require.NoError(t, err)

err = os.WriteFile(interpreterPath, []byte(""), 0755)
require.NoError(t, err)

err = os.WriteFile(filepath.Join(dir, "pyvenv.cfg"), []byte(""), 0755)
require.NoError(t, err)

executable, err := DetectVEnvExecutable(dir)

assert.NoError(t, err)
assert.Equal(t, interpreterPath, executable)
}

func TestDetectVEnvExecutable_badLayout(t *testing.T) {
dir := t.TempDir()

_, err := DetectVEnvExecutable(dir)

assert.Errorf(t, err, "can't find %q, check if virtualenv is created", interpreterPath(dir))
}

func interpreterPath(venvPath string) string {
if runtime.GOOS == "windows" {
return filepath.Join(venvPath, "Scripts", "python3.exe")
} else {
return filepath.Join(venvPath, "bin", "python3")
}
}

0 comments on commit 44902fa

Please sign in to comment.