Skip to content
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
2 changes: 1 addition & 1 deletion cmd/project/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewInitCommand(clients *shared.ClientFactory) *cobra.Command {
"Installs your project dependencies when supported:",
"- Deno: Supported",
"- Node.js: Supported",
"- Python: Unsupported",
"- Python: Supported",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐍 praise: Such a nice diff to find!

"",
"Adds an existing app to your project (optional):",
"- Prompts to add an existing app from app settings",
Expand Down
174 changes: 127 additions & 47 deletions internal/runtime/python/python.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package python

import (
"bytes"
"context"
_ "embed"
"fmt"
Expand Down Expand Up @@ -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
Copy link
Member Author

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 hookExecutor so that we can mock the exec calls during tests. This is how we also handle npm install.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mwbrooks This is a solid approach! IIRC the default hook execution protocol is used here since we don't have hooks loaded at this point, and the testing approach is great!

I'm hesitant on the CreateVirtualEnvironment hook name right now - it's almost an extension of a shared InstallDependencies to me. Rambles but this isn't exposed in configurations so for iteration ongoing 📚

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
Copy link
Member

Choose a reason for hiding this comment

The 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:

initializeAndInstallRequirementsTxt

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")
Expand Down Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Improving error messages when there is an error adding slack-cli-hooks to pyproject.toml.

}

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.
Expand All @@ -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 = ["
Expand Down Expand Up @@ -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
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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")
Copy link
Member

Choose a reason for hiding this comment

The 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 {
Expand All @@ -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
Expand Down
Loading
Loading