Merge pull request #258 from rstudio/mm-python-on-path

Python on path

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"]

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"}

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)

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)
+}