Skip to content

Commit

Permalink
libs/python: Remove DetectInterpreters (#2234)
Browse files Browse the repository at this point in the history
## Changes
- Remove DetectInterpreters from DetectExecutable call: python3 or
python should always be on on the PATH. We don't need to detect
non-standard situations like python3.10 is present but python3 is not.
- I moved DetectInterpreters to cmd/labs where it is still used.

This is a follow up to #2034

## Tests
Existing tests.
  • Loading branch information
denik authored Jan 27, 2025
1 parent 4595c6f commit 65fbbd9
Show file tree
Hide file tree
Showing 20 changed files with 8 additions and 37 deletions.
3 changes: 1 addition & 2 deletions cmd/labs/project/installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/databricks/cli/libs/databrickscfg/profile"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/process"
"github.com/databricks/cli/libs/python"
"github.com/databricks/databricks-sdk-go"
"github.com/databricks/databricks-sdk-go/service/compute"
"github.com/databricks/databricks-sdk-go/service/sql"
Expand Down Expand Up @@ -223,7 +222,7 @@ func (i *installer) setupPythonVirtualEnvironment(ctx context.Context, w *databr
feedback := cmdio.Spinner(ctx)
defer close(feedback)
feedback <- "Detecting all installed Python interpreters on the system"
pythonInterpreters, err := python.DetectInterpreters(ctx)
pythonInterpreters, err := DetectInterpreters(ctx)
if err != nil {
return fmt.Errorf("detect: %w", err)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package python
package project

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//go:build unix

package python
package project

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//go:build windows

package python
package project

import (
"context"
Expand Down
22 changes: 1 addition & 21 deletions libs/python/detect.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,27 +39,7 @@ func DetectExecutable(ctx context.Context) (string, error) {
//
// See https://github.com/pyenv/pyenv#understanding-python-version-selection

out, err := exec.LookPath(GetExecutable())

// most of the OS'es have python3 in $PATH, but for those which don't,
// we perform the latest version lookup
if err != nil && !errors.Is(err, exec.ErrNotFound) {
return "", err
}
if out != "" {
return out, nil
}
// otherwise, detect all interpreters and pick the least that satisfies
// minimal version requirements
all, err := DetectInterpreters(ctx)
if err != nil {
return "", err
}
interpreter, err := all.AtLeast("3.8")
if err != nil {
return "", err
}
return interpreter.Path, nil
return exec.LookPath(GetExecutable())
}

// DetectVEnvExecutable returns the path to the python3 executable inside venvPath,
Expand Down
12 changes: 2 additions & 10 deletions libs/python/detect_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,16 @@ func TestDetectsViaPathLookup(t *testing.T) {
assert.NotEmpty(t, py)
}

func TestDetectsViaListing(t *testing.T) {
t.Setenv("PATH", "testdata/other-binaries-filtered")
ctx := context.Background()
py, err := DetectExecutable(ctx)
assert.NoError(t, err)
assert.Equal(t, "testdata/other-binaries-filtered/python3.10", py)
}

func TestDetectFailsNoInterpreters(t *testing.T) {
t.Setenv("PATH", "testdata")
ctx := context.Background()
_, err := DetectExecutable(ctx)
assert.Equal(t, ErrNoPythonInterpreters, err)
assert.Error(t, err)
}

func TestDetectFailsNoMinimalVersion(t *testing.T) {
t.Setenv("PATH", "testdata/no-python3")
ctx := context.Background()
_, err := DetectExecutable(ctx)
assert.EqualError(t, err, "cannot find Python greater or equal to v3.8.0")
assert.Error(t, err)
}
2 changes: 1 addition & 1 deletion libs/python/detect_win_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ func TestDetectFailsNoInterpreters(t *testing.T) {
t.Setenv("PATH", "testdata")
ctx := context.Background()
_, err := DetectExecutable(ctx)
assert.ErrorIs(t, err, ErrNoPythonInterpreters)
assert.Error(t, err)
}

0 comments on commit 65fbbd9

Please sign in to comment.