diff --git a/extensions/jupyterlab/connect_jupyterlab/connect_jupyterlab/handlers.py b/extensions/jupyterlab/connect_jupyterlab/connect_jupyterlab/handlers.py index 5650ee411..02c94dca8 100644 --- a/extensions/jupyterlab/connect_jupyterlab/connect_jupyterlab/handlers.py +++ b/extensions/jupyterlab/connect_jupyterlab/connect_jupyterlab/handlers.py @@ -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 @@ -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() @@ -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"] diff --git a/internal/environment/python.go b/internal/environment/python.go index 97964f7f2..9f26fa428 100644 --- a/internal/environment/python.go +++ b/internal/environment/python.go @@ -3,6 +3,7 @@ package environment import ( "bytes" "fmt" + "io/fs" "os/exec" "strings" @@ -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 @@ -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"} diff --git a/internal/environment/python_test.go b/internal/environment/python_test.go index 6d0c5660c..29aa6d0c1 100644 --- a/internal/environment/python_test.go +++ b/internal/environment/python_test.go @@ -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) @@ -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) @@ -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) diff --git a/internal/util/process.go b/internal/util/process.go new file mode 100644 index 000000000..252a64061 --- /dev/null +++ b/internal/util/process.go @@ -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) +}