feat: support pyproject.toml and uv in python extension (#3118)#3176
feat: support pyproject.toml and uv in python extension (#3118)#3176deepshekhardas wants to merge 1 commit intotriggerdotdev:mainfrom
Conversation
|
|
Hi @deepshekhardas, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe changes add support for pyproject.toml-based Python dependency management to the extension. Two new optional fields are introduced to PythonOptions: Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 Pre-existing: requirementsFile warning always fires spuriously
In onBuildComplete at lines 114-119, there's a warning logged when both requirementsFile and requirements are set. However, the constructor at lines 51-58 always sets this.options.requirements from the file contents when requirementsFile is provided. The assert at line 46-48 prevents both from being user-specified simultaneously. This means the warning at line 115-118 will fire every time requirementsFile is used, even though the requirements array was set by the constructor itself, not by the user. The warning message ('requirements will be ignored') is misleading in this context. This is pre-existing and not introduced by this PR.
(Refers to lines 114-119)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🚩 Missing changeset for public package modification
The CLAUDE.md and CONTRIBUTING.md both state that modifications to any public package (packages/*) require a changeset via pnpm run changeset:add. This PR modifies packages/python/src/extension.ts (adding new pyprojectFile and useUv options) but no changeset file was added to .changeset/. This is a release management concern — without a changeset, this feature won't appear in the changelog and the package version won't be bumped on release.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (this.options.pyprojectFile) { | ||
| assert( | ||
| fs.existsSync(this.options.pyprojectFile), | ||
| `pyproject.toml not found: ${this.options.pyprojectFile}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔴 Missing mutual exclusion validation for pyprojectFile against requirements/requirementsFile
The constructor validates that requirements and requirementsFile cannot both be specified (line 46-48), but the new pyprojectFile option has no such validation. A user can pass { pyprojectFile: "pyproject.toml", requirements: ["numpy"] } or { pyprojectFile: "pyproject.toml", requirementsFile: "requirements.txt" } without error. In onBuildComplete, the if/else if/else if chain at lines 114-199 silently picks one option and ignores the other — requirementsFile wins over pyprojectFile, which wins over requirements. The user gets no error or warning that their configuration is conflicting and one dependency source is being dropped.
Prompt for agents
In packages/python/src/extension.ts, in the constructor (around lines 45-66), add assertions that prevent pyprojectFile from being used together with requirements or requirementsFile. Before the existing assert on line 46, or after it, add:
assert(
!(this.options.pyprojectFile && this.options.requirements),
"Cannot specify both pyprojectFile and requirements"
);
assert(
!(this.options.pyprojectFile && this.options.requirementsFile),
"Cannot specify both pyprojectFile and requirementsFile"
);
This matches the existing validation pattern at lines 46-48 and ensures users get a clear error when they provide conflicting dependency sources.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Add a layer to the build that installs the dependencies | ||
| context.addLayer({ | ||
| id: "python-dependencies", | ||
| image: { | ||
| instructions: splitAndCleanComments(` | ||
| # Copy the pyproject file | ||
| COPY ${this.options.pyprojectFile} . | ||
| # Install dependencies | ||
| ${this.options.useUv ? "RUN uv pip install ." : "RUN pip install ."} | ||
| `), | ||
| }, | ||
| deploy: { |
There was a problem hiding this comment.
🚩 pip install . with only pyproject.toml may fail at build time
When pyprojectFile is used (lines 149-174), only the pyproject.toml file is copied into the container via addAdditionalFilesToBuild, and then pip install . (or uv pip install .) is executed. The pip install . command attempts to build and install the current directory as a Python package, which typically requires the full source tree (not just pyproject.toml). If the pyproject.toml only declares dependencies without a buildable package, or if the source files aren't present, the build will fail. Users would need to also use the scripts option to copy their Python source files for this to work. This isn't strictly a code bug since the Docker build would fail with a clear error, but it may warrant documentation or a different pip invocation (e.g., pip install with dependency extraction from pyproject.toml).
(Refers to lines 160-174)
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Modern Python projects are increasingly adopting
pyproject.tomlfor dependency management and project configuration, often using faster tools likeuv. The current Trigger.dev Python extension only supportsrequirements.txtor inline requirements.This PR adds support for
pyproject.tomland the optional use of theuvpackage manager during the build process.Closes #3118
Changes
pyprojectFileanduseUvtoPythonOptions.uvifuseUvis set totrue.pyproject.tomlfor dependency installation (pip install .oruv pip install .).requirements.txtor inline requirements withuvsupport if enabled.Testing
uvinstallation and usage instructions are correctly injected into the build layers.