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.

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

Python on path #258

Merged
merged 9 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import shlex
import subprocess
from urllib.parse import urlparse
from typing import Set, Dict, Tuple
from typing import Set, Dict, Tuple, Any

from jupyter_server.base.handlers import APIHandler
from jupyter_server.utils import url_path_join
Expand All @@ -14,7 +14,7 @@
from tornado.httputil import HTTPServerRequest
from tornado.web import authenticated

base_url = None
base_url: str = ""
EXECUTABLE = "connect-client"

known_ports: Set[int] = set()
Expand All @@ -26,7 +26,7 @@ class PublishHandler(APIHandler):
def post(self) -> None:
"""post initiates the publishing process. Details TBD."""
self.log.info("Launching publishing UI")
data: Dict[str, str] = self.get_json_body()
data: Any = self.get_json_body()
notebookPath = os.path.abspath(data["notebookPath"])
pythonPath = data["pythonPath"]
pythonVersion = data["pythonVersion"]
Expand Down
51 changes: 46 additions & 5 deletions internal/environment/python.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package environment
import (
"bytes"
"fmt"
"io/fs"
"os/exec"
"strings"

Expand Down Expand Up @@ -54,18 +55,54 @@ func NewPythonInspector(projectDir util.Path, pythonPath util.Path, log logging.
}
}

func (i *defaultPythonInspector) getPythonExecutable() string {
func (i *defaultPythonInspector) validatePythonExecutable(pythonExecutable string) error {
args := []string{"--version"}
_, err := i.executor.runPythonCommand(pythonExecutable, args)
return err
}

func (i *defaultPythonInspector) getPythonExecutable(exec util.PathLooker) (string, error) {
if i.pythonPath.Path() != "" {
// User-provided python executable
return i.pythonPath.Path()
exists, err := i.pythonPath.Exists()
if err != nil {
return "", err
}
if exists {
return i.pythonPath.Path(), nil
}
return "", fmt.Errorf(
"cannot find the specified Python executable %s: %w",
i.pythonPath, fs.ErrNotExist)
} else {
// Use whatever is on PATH
return "python3"
path, err := exec.LookPath("python3")
if err == nil {
// Ensure the Python is actually runnable. This is especially
// needed on Windows, where `python3` is (by default)
// an app execution alias. Also, installing Python from
// python.org does not disable the built-in app execution aliases.
err = i.validatePythonExecutable(path)
}
if err != nil {
path, err = exec.LookPath("python")
if err == nil {
err = i.validatePythonExecutable(path)
}
}
if err != nil {
return "", err
}
return path, nil
}
}

func (i *defaultPythonInspector) GetPythonVersion() (string, error) {
pythonExecutable := i.getPythonExecutable()
pythonExecutable, err := i.getPythonExecutable(util.NewPathLooker())
if err != nil {
return "", err
}
i.log.Info("Running Python", "python", pythonExecutable)
args := []string{
`-E`, // ignore python-specific environment variables
`-c`, // execute the next argument as python code
Expand All @@ -90,7 +127,11 @@ func (i *defaultPythonInspector) GetPythonRequirements() ([]byte, error) {
i.log.Info("Using Python packages", "source", requirementsFilename)
return requirementsFilename.ReadFile()
}
pythonExecutable := i.getPythonExecutable()
pythonExecutable, err := i.getPythonExecutable(util.NewPathLooker())
if err != nil {
return nil, err
}
i.log.Info("Running Python", "python", pythonExecutable)
source := fmt.Sprintf("'%s -m pip freeze'", pythonExecutable)
i.log.Info("Using Python packages", "source", source)
args := []string{"-m", "pip", "freeze"}
Expand Down
88 changes: 85 additions & 3 deletions internal/environment/python_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (s *PythonSuite) TestGetPythonVersionFromPATH() {
log := logging.New()
inspector := NewPythonInspector(util.Path{}, util.Path{}, log)
executor := NewMockPythonExecutor()
executor.On("runPythonCommand", "python3", mock.Anything).Return([]byte("3.10.4"), nil)
executor.On("runPythonCommand", mock.Anything, mock.Anything).Return([]byte("3.10.4"), nil)
inspector.executor = executor
version, err := inspector.GetPythonVersion()
s.Nil(err)
Expand All @@ -93,10 +93,13 @@ func (s *PythonSuite) TestGetPythonVersionFromPATH() {
}

func (s *PythonSuite) TestGetPythonVersionFromRealDefaultPython() {
// This test can only run if python3 is on the PATH.
// This test can only run if python3 or python is on the PATH.
_, err := exec.LookPath("python3")
if err != nil {
s.T().Skip("python3 isn't available on PATH")
_, err := exec.LookPath("python")
if err != nil {
s.T().Skip("This test requires python or python3 to be available on PATH")
}
}
log := logging.New()
inspector := NewPythonInspector(util.Path{}, util.Path{}, log)
Expand All @@ -105,6 +108,85 @@ func (s *PythonSuite) TestGetPythonVersionFromRealDefaultPython() {
s.True(strings.HasPrefix(version, "3."))
}

type mockPythonExecutor struct {
mock.Mock
}

var _ pythonExecutor = &mockPythonExecutor{}

func (m *mockPythonExecutor) runPythonCommand(pythonExecutable string, args []string) ([]byte, error) {
mockArgs := m.Called(pythonExecutable, args)
out := mockArgs.Get(0)
if out != nil {
return out.([]byte), mockArgs.Error(1)
} else {
return nil, mockArgs.Error(1)
}
}

func (s *PythonSuite) TestGetPythonExecutableFallbackPython() {
// python3 does not exist
// python exists and is runnable
log := logging.New()
executor := &mockPythonExecutor{}
executor.On("runPythonCommand", "/some/python", mock.Anything).Return(nil, nil)
inspector := &defaultPythonInspector{
executor: executor,
log: log,
}

exec := util.NewMockPathLooker()
exec.On("LookPath", "python3").Return("", os.ErrNotExist)
exec.On("LookPath", "python").Return("/some/python", nil)
executable, err := inspector.getPythonExecutable(exec)
s.NoError(err)
s.Equal("/some/python", executable)
}

func (s *PythonSuite) TestGetPythonExecutablePython3NotRunnable() {
// python3 exists but is not runnable
// python exists and is runnable
log := logging.New()
executor := &mockPythonExecutor{}
testError := errors.New("exit status 9009")
executor.On("runPythonCommand", "/some/python3", mock.Anything).Return(nil, testError)
executor.On("runPythonCommand", "/some/python", mock.Anything).Return(nil, nil)

inspector := &defaultPythonInspector{
executor: executor,
log: log,
}

exec := util.NewMockPathLooker()
exec.On("LookPath", "python3").Return("/some/python3", nil)
exec.On("LookPath", "python").Return("/some/python", nil)
executable, err := inspector.getPythonExecutable(exec)
s.NoError(err)
s.Equal("/some/python", executable)
}

func (s *PythonSuite) TestGetPythonExecutableNoRunnablePython() {
// python3 exists but is not runnable
// python exists but is not runnable
log := logging.New()
executor := &mockPythonExecutor{}
testError := errors.New("exit status 9009")
executor.On("runPythonCommand", "/some/python3", mock.Anything).Return(nil, testError)
executor.On("runPythonCommand", "/some/python", mock.Anything).Return(nil, testError)

inspector := &defaultPythonInspector{
executor: executor,
log: log,
}

exec := util.NewMockPathLooker()
exec.On("LookPath", "python3").Return("/some/python3", nil)
exec.On("LookPath", "python").Return("/some/python", nil)
executable, err := inspector.getPythonExecutable(exec)
s.NotNil(err)
s.Equal("", executable)
}

func (s *PythonSuite) TestGetRequirementsFromFile() {
baseDir, err := util.Getwd(afero.NewMemMapFs())
s.Nil(err)
Expand Down
36 changes: 36 additions & 0 deletions internal/util/process.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package util

// Copyright (C) 2023 by Posit Software, PBC.

import (
"os/exec"

"github.com/stretchr/testify/mock"
)

type PathLooker interface {
LookPath(name string) (string, error)
}

type defaultPathLooker struct{}

func NewPathLooker() PathLooker {
return &defaultPathLooker{}
}

func (p *defaultPathLooker) LookPath(name string) (string, error) {
return exec.LookPath(name)
}

type mockPathLooker struct {
mock.Mock
}

func NewMockPathLooker() *mockPathLooker {
return &mockPathLooker{}
}

func (m *mockPathLooker) LookPath(name string) (string, error) {
args := m.Called(name)
return args.String(0), args.Error(1)
}