-
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
Changes from all commits
9d6e1d9
665e23c
9008d02
51030ee
6e6ba42
cf35c5c
bff1d8c
b0f0050
e641b20
4324103
3a76ba8
180d499
409dc41
60f4290
cbfbbe1
abe12d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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) | ||
|
Comment on lines
+98
to
+108
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: While it's verbose, we use the
Member
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. @mwbrooks This is a solid approach! IIRC the I'm hesitant on the |
||
| 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 | ||
|
Member
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: I found the test cases confusing for these "install" functions that are also updating the requirements. Not a blocker for this PR, but we might change this to be something as: If changes are ongoing? Apologies for calling this out in an unrelated PR too! |
||
| 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"))) | ||
|
Comment on lines
-222
to
-239
Member
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. 🪓 praise: Farewell setup steps! I notice #347 brings changes to make the activation step automatic too! Really neat! |
||
| // 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") | ||
|
Member
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. 🌟 praise: Debugging the start of something and printing the end is a super fascinating pattern! |
||
| 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 | ||
|
|
||
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.
🐍 praise: Such a nice
diffto find!