Skip to content

Conversation

@rshade
Copy link
Contributor

@rshade rshade commented Dec 19, 2025

…tall

When PythonLinks are specified, the framework now handles Python dependency installation itself instead of relying on pulumi install. This avoids PEP 668 issues on systems with externally-managed Python.

The new flow:

  1. Creates a venv at venv/ (Pulumi's default location)
  2. Upgrades pip using the venv's Python (not system Python)
  3. Installs local packages via pip install -e
  4. Installs requirements.txt dependencies
  5. Runs pulumi plugin install for Pulumi plugins

Users must configure their Pulumi.yaml with:
runtime:
name: python
options:
toolchain: pip
virtualenv: venv

Fixes the issue where PythonLink required SkipInstall() to work.

…tall

When PythonLinks are specified, the framework now handles Python
dependency installation itself instead of relying on `pulumi install`.
This avoids PEP 668 issues on systems with externally-managed Python.

The new flow:
1. Creates a venv at `venv/` (Pulumi's default location)
2. Upgrades pip using the venv's Python (not system Python)
3. Installs local packages via `pip install -e`
4. Installs requirements.txt dependencies
5. Runs `pulumi plugin install` for Pulumi plugins

Users must configure their Pulumi.yaml with:
  runtime:
    name: python
    options:
      toolchain: pip
      virtualenv: venv

Fixes the issue where PythonLink required SkipInstall() to work.
Copilot AI review requested due to automatic review settings December 19, 2025 20:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes PythonLink functionality to work without requiring SkipInstall() by implementing a custom Python dependency installation flow that bypasses pulumi install. This solves PEP 668 issues on systems with externally-managed Python environments.

Key Changes:

  • New installPythonDependencies() method creates a venv, installs local packages, requirements.txt dependencies, and Pulumi plugins
  • Automatic validation of Pulumi.yaml configuration with helpful error messages when virtualenv settings are missing
  • Comprehensive documentation updates explaining the new workflow and required Pulumi.yaml configuration

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pulumitest/pulumiTest.go Adds core implementation: installPythonDependencies(), YAML validation functions, and integration with pulumiTestInit()
pulumitest/newStack.go Removes old PythonLink installation logic (now handled centrally in pulumiTestInit())
pulumitest/pulumiTest_test.go Adds TestPythonLinkWithInstall() to verify PythonLink works without SkipInstall
pulumitest/testdata/python_with_local_pkg/Pulumi.yaml Updates to include required runtime configuration (virtualenv and toolchain settings)
pulumitest/testdata/python_with_local_pkg/requirements.txt Adds pulumi SDK and local test package to verify dependency installation order
pulumitest/README.md Documents the new PythonLink workflow, installation steps, and required Pulumi.yaml configuration
CONTRIBUTING.md Updates troubleshooting guide with new PythonLink behavior and configuration requirements

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pipPath = filepath.Join(venvPath, "bin", "pip")
venvPython = filepath.Join(venvPath, "bin", "python")
}

Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The platform detection logic checks for the existence of pip.exe to determine if the system is Windows. However, this check happens after the venv is created but doesn't verify that the venv was created successfully with the expected structure. If the venv creation succeeds but doesn't create the expected files (e.g., due to an unusual Python installation), both the Windows and Unix paths will be set, but the files might not exist.

Additionally, if the venv exists from a previous run but was created with a different platform or is corrupted, the detection might choose the wrong path.

Consider adding error handling to verify that the selected pipPath and venvPython actually exist before proceeding with the installation steps.

Suggested change
// Verify that the selected pip and python executables actually exist
if _, err := os.Stat(pipPath); err != nil {
ptFatalF(t, "virtual environment pip executable not found at %s: %s", pipPath, err)
}
if _, err := os.Stat(venvPython); err != nil {
ptFatalF(t, "virtual environment python executable not found at %s: %s", venvPython, err)
}

Copilot uses AI. Check for mistakes.
Comment on lines +243 to +252
// Step 1: Create venv if it doesn't exist
if _, err := os.Stat(venvPath); os.IsNotExist(err) {
cmd := exec.Command(pythonCmd, "-m", "venv", venvPath)
cmd.Dir = pt.workingDir
ptLogF(t, "creating virtual environment: %s", cmd)
out, err := cmd.CombinedOutput()
if err != nil {
ptFatalF(t, "failed to create virtual environment: %s\n%s", err, out)
}
}
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The venv creation is skipped if the directory already exists (using os.IsNotExist). However, if the venv directory exists but is incomplete or corrupted (e.g., from a previous failed test run or if the venv was created with a different Python version), the installation steps will proceed with the potentially broken venv.

Consider adding validation to check if the venv is functional before using it, or provide a way to force recreation of the venv if needed. Alternatively, the error handling in subsequent steps (pip upgrade, package installation) should provide clear error messages that help users understand if the issue is with a corrupted venv.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant