fix(python-sdk): strip uppercase/mixed-case AS stage alias in from_dockerfile - #1788
fix(python-sdk): strip uppercase/mixed-case AS stage alias in from_dockerfile#1788eeshsaxena wants to merge 1 commit into
Conversation
…ckerfile parse_dockerfile checked base_image.lower() for " as " but then split the original-case string on the lowercase " as ", so the canonical uppercase form FROM node:24 AS builder left the base image as "node:24 AS builder" (a broken image reference); only a lowercase " as " was stripped. Match AS case-insensitively. Adds sync and async regression tests.
|
We require contributors to sign our Contributor License Agreement, and we don't have @eeshsaxena on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
|
|
Concrete impact: a single-stage Dockerfile like FROM node:24 AS builder (the canonical uppercase form the Docker docs use) currently yields the base image "node:24 AS builder", which is not a valid image reference, so from_dockerfile builds a broken template. Lowercase happened to work, which is why it slipped through. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
TASTE.md review — no violations found (0).
Principles checked against the changed lines: parity across JS / sync Python / async Python (T-1, T-2), builder error types (T-42), Python option/result typing (T-16, T-17), docstrings (T-69, T-71). The fix is confined to parse_dockerfile's internal base-image normalization and changes no public surface, so most of the API-shape rules don't apply.
- T-1 / T-2 parity: verified the JS parser is not affected by the same bug —
packages/js-sdk/src/template/dockerfileParser.tstakesargumentsData[0].getValue()from theFROMinstruction, so the stage alias is a separate argument and never lands inbaseImage. No JS-side change is owed here. The sync and async Python test mirrors are both updated identically, as T-2 requires. - T-42 (builder failures raise
BuildException, not bareValueError) is violated by the surroundingraise ValueError(...)lines in this file, but those are pre-existing and untouched by this PR, so out of scope.
Two non-TASTE notes, take or leave: the new comments in the source and both tests narrate the previous bug ("Previously only a lowercase as was stripped") rather than describing the code — that history reads better in the PR description than in the file; and per AGENTS.md a user-visible python-sdk fix normally wants a changeset in .changeset/, which this PR doesn't add.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70db30a044
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| base_image = re.split( | ||
| r"\s+as\s+", base_image, maxsplit=1, flags=re.IGNORECASE | ||
| )[0].strip() |
There was a problem hiding this comment.
This changes the user-visible behavior of Template.from_dockerfile, but the commit contains no .changeset entry for @e2b/python-sdk. In the checked release workflow, Python publication is gated through is_release_for_package.sh, which returns true only when a changeset names that package, so this fix will not bump or publish the Python SDK until it happens to be included in an unrelated release. Add a patch changeset for @e2b/python-sdk.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
1 participant
What
parse_dockerfile(used byTemplate.from_dockerfile) strips a stage alias from the base image, but only when it is written as lowercaseas:The guard lowercases the string, but the
splitruns on the original-case value with a lowercase delimiter. So the canonical, uppercase Docker form is not stripped:ASis case-insensitive in Dockerfiles (and the docs use uppercase), so this is the common case.Fix
Match
AScase-insensitively:Verified against the real
parse_dockerfile:FROM node:24 AS builder,as builder,As Build, andFROM node:24all resolve tonode:24. Added sync + async regression tests.How the code was written
Authored with the help of a coding agent; I verified the fix by running the actual
parse_dockerfileand can explain the change.