Skip to content

fix(integration-tests): fail the Python build when pip fails - #1468

Open
lym953 wants to merge 3 commits into
mainfrom
yiming.luo/build-python-fail-on-pip-error
Open

lym953 wants to merge 3 commits into
mainfrom
yiming.luo/build-python-fail-on-pip-error

Conversation

@lym953

@lym953 lym953 commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

When pip install fails for a Python test function, the "build python lambdas" job still passes. The deployed function then has only its handler file and no dependencies, and the failure shows up later as a test that gets no data.

This happened on #1461. pip stopped with a dependency conflict:

ERROR: Cannot install -r requirements.txt (line 2) and opentelemetry-api==1.45.1 because these package versions have conflicting dependencies.

The job then printed "otlp-python built successfully", with only lambda_function.py in the package. Later, the otlp integration suite failed because the Python function sent no trace.

Cause

The script sets set -e, but the build loop calls the build function inside an if condition. Bash turns off set -e in that context, so the script continues after pip fails. The check after the install step looks only for a non-empty package/ directory, and the script copies the .py files into that directory first. So the check always passes.

This PR

Checks the exit status of the docker run step that runs pip. When that step fails, the function prints the exit status and returns 1. The job then fails and lists the function. The message names the docker run step, not pip, because Docker can also fail before pip starts. An example is a Docker daemon that is not running.

Testing

Steps

  1. Put a test function with conflicting requirements in a copy of the script layout.
  2. Replaced docker on the PATH with a stub that exits 1, and ran the script from main and from this branch.
  3. Replaced the stub with one that exits 125, and ran this branch again.
  4. Replaced the stub with one that writes a file to package/ and exits 0, and ran this branch again.

Result

Case Exit code before Exit code after
pip fails 0, "All Python Lambda builds completed successfully!" 1, "Build failed: docker run ... pip install exited with status 1"
Docker exits 125 Not run 1, "Build failed: docker run ... pip install exited with status 125"
pip passes Not run 0

I did not run the real Docker image. CI runs it.

🤖 Generated with Claude Code

The build function runs in an if condition, where set -e is off, so a pip failure produced a package with only the handler file and a passing build.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lym953
lym953 marked this pull request as ready for review October 28, 2026 16:06
Copilot AI balanced review requested due to automatic review settings October 28, 2026 16:06
@lym953
lym953 requested a review from a team as a code owner October 28, 2026 16:06
@lym953
lym953 requested a review from clifordshelton October 28, 2026 16:06
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-02T19:29:45.744507Z 86e7aef New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The script reports Docker startup failures as pip errors, which can misdirect troubleshooting.

1 open issue
What changed in this PR

This PR makes Python integration-test Lambda builds fail when dependency installation fails, preventing incomplete packages from causing misleading downstream test failures.

Changes:

  • Explicitly checks the Docker command’s exit status rather than relying on set -e.
  • Reports installation failure and returns a nonzero status to the build loop.
  • Documents why the existing package-directory check cannot detect pip failures.
File Description
integration-tests/​scripts/​build-python.sh Adds explicit failure handling around Docker-based dependency installation.

Review effort: Balanced

@datadog-official

datadog-official Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Pipelines

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 72f39db | Docs | Give us feedback!

lym953 and others added 2 commits November 2, 2026 14:28
… message

Docker can fail before pip starts, so print the step and its exit status
instead of blaming pip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

2 participants