-
Notifications
You must be signed in to change notification settings - Fork 28
feat(python): add support for creating python .venv and installing dependencies #346
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
Open
mwbrooks
wants to merge
12
commits into
main
Choose a base branch
from
mwbrooks-python-venv
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
9d6e1d9
feat(python): add support for creating and activating venv
mwbrooks 665e23c
fix(python): install pyproject.toml before requirements.txt
mwbrooks 9008d02
chore(cmd): remove venv detail from init command description
mwbrooks 51030ee
fix(python): use correct python executable on Windows for venv creation
mwbrooks 6e6ba42
chore(python): add comment explaining venv activation is unnecessary …
mwbrooks cf35c5c
chore(python): fix gofmt formatting in python_test.go
mwbrooks bff1d8c
chore(python): move venv creation message to debug output
mwbrooks b0f0050
chore(python): move dependency install messages to debug output
mwbrooks e641b20
chore(python): shorten install success messages and highlight filenames
mwbrooks 4324103
fix(python): improve error handling for dependency file updates
mwbrooks 3a76ba8
test: remove skipped test
mwbrooks 180d499
fix(python): use HookExecutor for venv and pip commands to fix tests
mwbrooks File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| package python | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| _ "embed" | ||
| "fmt" | ||
|
|
@@ -62,6 +63,80 @@ func (p *Python) IgnoreDirectories() []string { | |
| return []string{} | ||
| } | ||
|
|
||
| // getVenvPath returns the path to the virtual environment directory | ||
| func getVenvPath(projectDirPath string) string { | ||
| return filepath.Join(projectDirPath, ".venv") | ||
| } | ||
|
|
||
| // getPythonExecutable returns the Python executable name for the current OS | ||
| func getPythonExecutable() string { | ||
| if runtime.GOOS == "windows" { | ||
| return "python" | ||
| } | ||
| return "python3" | ||
| } | ||
|
|
||
| // getPipExecutable returns the path to the pip executable in the virtual environment | ||
| func getPipExecutable(venvPath string) string { | ||
| if runtime.GOOS == "windows" { | ||
| return filepath.Join(venvPath, "Scripts", "pip.exe") | ||
| } | ||
| return filepath.Join(venvPath, "bin", "pip") | ||
| } | ||
|
|
||
| // venvExists checks if a virtual environment exists at the given path | ||
| func venvExists(fs afero.Fs, venvPath string) bool { | ||
| pipPath := getPipExecutable(venvPath) | ||
| if _, err := fs.Stat(pipPath); err == nil { | ||
| return true | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // createVirtualEnvironment creates a Python virtual environment | ||
| func createVirtualEnvironment(ctx context.Context, projectDirPath string, hookExecutor hooks.HookExecutor) error { | ||
| hookScript := hooks.HookScript{ | ||
| Name: "CreateVirtualEnvironment", | ||
| Command: fmt.Sprintf("%s -m venv .venv", getPythonExecutable()), | ||
| } | ||
| stdout := bytes.Buffer{} | ||
| hookExecOpts := hooks.HookExecOpts{ | ||
| Hook: hookScript, | ||
| Stdout: &stdout, | ||
| Directory: projectDirPath, | ||
| } | ||
| _, err := hookExecutor.Execute(ctx, hookExecOpts) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create virtual environment: %w\nOutput: %s", err, stdout.String()) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // runPipInstall runs pip install with the given arguments. | ||
| // The venv does not need to be activated because pip is invoked by its full | ||
| // path inside the venv, which ensures packages are installed into the venv's | ||
| // site-packages directory. | ||
| func runPipInstall(ctx context.Context, venvPath string, projectDirPath string, hookExecutor hooks.HookExecutor, args ...string) (string, error) { | ||
| pipPath := getPipExecutable(venvPath) | ||
| cmdArgs := append([]string{pipPath, "install"}, args...) | ||
| hookScript := hooks.HookScript{ | ||
| Name: "InstallProjectDependencies", | ||
| Command: strings.Join(cmdArgs, " "), | ||
| } | ||
| stdout := bytes.Buffer{} | ||
| hookExecOpts := hooks.HookExecOpts{ | ||
| Hook: hookScript, | ||
| Stdout: &stdout, | ||
| Directory: projectDirPath, | ||
| } | ||
| _, err := hookExecutor.Execute(ctx, hookExecOpts) | ||
| output := stdout.String() | ||
| if err != nil { | ||
| return output, fmt.Errorf("pip install failed: %w", err) | ||
| } | ||
| return output, nil | ||
| } | ||
|
|
||
| // installRequirementsTxt handles adding slack-cli-hooks to requirements.txt | ||
| func installRequirementsTxt(fs afero.Fs, projectDirPath string) (output string, err error) { | ||
| requirementsFilePath := filepath.Join(projectDirPath, "requirements.txt") | ||
|
|
@@ -128,18 +203,18 @@ func installPyProjectToml(fs afero.Fs, projectDirPath string) (output string, er | |
| projectSection, exists := config["project"] | ||
| if !exists { | ||
| err := fmt.Errorf("pyproject.toml missing project section") | ||
| return fmt.Sprintf("Error: %s", err), err | ||
| return fmt.Sprintf("Error updating pyproject.toml: %s", err), err | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: Improving error messages when there is an error adding |
||
| } | ||
|
|
||
| projectMap, ok := projectSection.(map[string]interface{}) | ||
| if !ok { | ||
| err := fmt.Errorf("pyproject.toml project section is not a valid format") | ||
| return fmt.Sprintf("Error: %s", err), err | ||
| return fmt.Sprintf("Error updating pyproject.toml: %s", err), err | ||
| } | ||
|
|
||
| if _, exists := projectMap["dependencies"]; !exists { | ||
| err := fmt.Errorf("pyproject.toml missing dependencies array") | ||
| return fmt.Sprintf("Error: %s", err), err | ||
| return fmt.Sprintf("Error updating pyproject.toml: %s", err), err | ||
| } | ||
|
|
||
| // Use string manipulation to add the dependency while preserving formatting. | ||
|
|
@@ -151,7 +226,7 @@ func installPyProjectToml(fs afero.Fs, projectDirPath string) (output string, er | |
|
|
||
| if len(matches) == 0 { | ||
| err := fmt.Errorf("pyproject.toml missing dependencies array") | ||
| return fmt.Sprintf("Error: %s", err), err | ||
| return fmt.Sprintf("Error updating pyproject.toml: %s", err), err | ||
| } | ||
|
|
||
| prefix := matches[1] // "...dependencies = [" | ||
|
|
@@ -189,8 +264,7 @@ func installPyProjectToml(fs afero.Fs, projectDirPath string) (output string, er | |
| return fmt.Sprintf("Updated pyproject.toml with %s", style.Highlight(slackCLIHooksPackageSpecifier)), nil | ||
| } | ||
|
|
||
| // InstallProjectDependencies is unsupported by Python because a virtual environment is required before installing the project dependencies. | ||
| // TODO(@mbrooks) - should we confirm that the project is using Bolt Python? | ||
| // InstallProjectDependencies creates a virtual environment and installs project dependencies. | ||
| func (p *Python) InstallProjectDependencies(ctx context.Context, projectDirPath string, hookExecutor hooks.HookExecutor, ios iostreams.IOStreamer, fs afero.Fs, os types.Os) (output string, err error) { | ||
| var outputs []string | ||
| var errs []error | ||
|
|
@@ -210,44 +284,26 @@ func (p *Python) InstallProjectDependencies(ctx context.Context, projectDirPath | |
| hasPyProjectToml = true | ||
| } | ||
|
|
||
| // Defer a function to transform the return values | ||
| defer func() { | ||
| // Manual steps to setup virtual environment and install dependencies | ||
| var activateVirtualEnv = "source .venv/bin/activate" | ||
| if runtime.GOOS == "windows" { | ||
| activateVirtualEnv = `.venv\Scripts\activate` | ||
| } | ||
|
|
||
| // Get the relative path to the project directory | ||
| var projectDirPathRel, _ = getProjectDirRelPath(os, os.GetExecutionDir(), projectDirPath) | ||
|
|
||
| outputs = append(outputs, fmt.Sprintf("Manually setup a %s", style.Highlight("Python virtual environment"))) | ||
| if projectDirPathRel != "." { | ||
| outputs = append(outputs, fmt.Sprintf(" Change into the project: %s", style.CommandText(fmt.Sprintf("cd %s%s", filepath.Base(projectDirPathRel), string(filepath.Separator))))) | ||
| } | ||
| outputs = append(outputs, fmt.Sprintf(" Create virtual environment: %s", style.CommandText("python3 -m venv .venv"))) | ||
| outputs = append(outputs, fmt.Sprintf(" Activate virtual environment: %s", style.CommandText(activateVirtualEnv))) | ||
|
|
||
| // Provide appropriate install command based on which file exists | ||
| if hasRequirementsTxt { | ||
| outputs = append(outputs, fmt.Sprintf(" Install project dependencies: %s", style.CommandText("pip install -r requirements.txt"))) | ||
| } | ||
| if hasPyProjectToml { | ||
| outputs = append(outputs, fmt.Sprintf(" Install project dependencies: %s", style.CommandText("pip install -e ."))) | ||
| } | ||
| // Ensure at least one dependency file exists | ||
| if !hasRequirementsTxt && !hasPyProjectToml { | ||
| err := fmt.Errorf("no Python dependency file found (requirements.txt or pyproject.toml)") | ||
| return fmt.Sprintf("Error: %s", err), err | ||
| } | ||
|
|
||
| outputs = append(outputs, fmt.Sprintf(" Learn more: %s", style.Underline("https://docs.python.org/3/tutorial/venv.html"))) | ||
| // Get virtual environment path | ||
| venvPath := getVenvPath(projectDirPath) | ||
|
|
||
| // Get first error or nil | ||
| var firstErr error | ||
| if len(errs) > 0 { | ||
| firstErr = errs[0] | ||
| // Create virtual environment if it doesn't exist | ||
| if !venvExists(fs, venvPath) { | ||
| ios.PrintDebug(ctx, "Creating Python virtual environment") | ||
| if err := createVirtualEnvironment(ctx, projectDirPath, hookExecutor); err != nil { | ||
| outputs = append(outputs, fmt.Sprintf("Error creating virtual environment: %s", err)) | ||
| return strings.Join(outputs, "\n"), err | ||
| } | ||
|
|
||
| // Update return value | ||
| output = strings.Join(outputs, "\n") | ||
| err = firstErr | ||
| }() | ||
| outputs = append(outputs, fmt.Sprintf("Created virtual environment at %s", style.Highlight(".venv"))) | ||
| } else { | ||
| outputs = append(outputs, fmt.Sprintf("Found existing virtual environment at %s", style.Highlight(".venv"))) | ||
| } | ||
|
|
||
| // Handle requirements.txt if it exists | ||
| if hasRequirementsTxt { | ||
|
|
@@ -267,14 +323,38 @@ func (p *Python) InstallProjectDependencies(ctx context.Context, projectDirPath | |
| } | ||
| } | ||
|
|
||
| // If neither file exists, return an error | ||
| if !hasRequirementsTxt && !hasPyProjectToml { | ||
| err := fmt.Errorf("no Python dependency file found (requirements.txt or pyproject.toml)") | ||
| errs = append(errs, err) | ||
| outputs = append(outputs, fmt.Sprintf("Error: %s", err)) | ||
| // Install dependencies using pip | ||
| // When both files exist, pyproject.toml is installed first to set up the project package | ||
| // and its declared dependencies. Then requirements.txt is installed second so its version | ||
| // pins take precedence, as it typically serves as the lockfile. | ||
| if hasPyProjectToml { | ||
| ios.PrintDebug(ctx, "Installing dependencies from pyproject.toml") | ||
| pipOutput, err := runPipInstall(ctx, venvPath, projectDirPath, hookExecutor, "-e", ".") | ||
| if err != nil { | ||
| errs = append(errs, err) | ||
| outputs = append(outputs, fmt.Sprintf("Error installing from pyproject.toml: %s\n%s", err, pipOutput)) | ||
| } else { | ||
| outputs = append(outputs, fmt.Sprintf("Installed dependencies from %s", style.Highlight("pyproject.toml"))) | ||
| } | ||
| } | ||
|
|
||
| return | ||
| if hasRequirementsTxt { | ||
| ios.PrintDebug(ctx, "Installing dependencies from requirements.txt") | ||
| pipOutput, err := runPipInstall(ctx, venvPath, projectDirPath, hookExecutor, "-r", "requirements.txt") | ||
| if err != nil { | ||
| errs = append(errs, err) | ||
| outputs = append(outputs, fmt.Sprintf("Error installing from requirements.txt: %s\n%s", err, pipOutput)) | ||
| } else { | ||
| outputs = append(outputs, fmt.Sprintf("Installed dependencies from %s", style.Highlight("requirements.txt"))) | ||
| } | ||
| } | ||
|
|
||
| // Return result | ||
| output = strings.Join(outputs, "\n") | ||
| if len(errs) > 0 { | ||
| return output, errs[0] | ||
| } | ||
| return output, nil | ||
| } | ||
|
|
||
| // Name prints the name of the runtime | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: While it's verbose, we use the
hookExecutorso that we can mock the exec calls during tests. This is how we also handlenpm install.