Conversation
- Replace pip with uv in Dockerfile to avoid pip's resolution-too-deep error - Bump dbt-core and all adapter lower bounds from >=0.20 to >=1.8 (pre-1.8 is EOL) Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
👋 @devin-ai-integration[bot] |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces Dockerfile's direct Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Docker as Docker build
participant Py as Python env
participant UV as uv (tool)
participant PyPI as PyPI
Dev->>Docker: docker build (with UV_VERSION)
Docker->>Py: create image base, install python deps
Docker->>Py: pip install "uv==${UV_VERSION}"
Py->>UV: uv available in environment
Docker->>UV: uv pip install --no-cache --system "/app[all]"
UV->>PyPI: fetch packages listed in `all` extras
PyPI-->>UV: return packages
UV-->>Py: install packages into system environment
Docker-->>Dev: image built with dependencies installed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Around line 21-22: Pin the `uv` package to a tested, specific version and
merge the two RUN steps into a single Docker layer; replace the loose "RUN pip
install uv" with a pinned installation (e.g., `pip install
uv==<tested-version>`) and correct and combine the second command (the erroneous
`RUN uv pip install --no-cache --system "/app[all]"`) so both installs run in
one RUN instruction to keep builds reproducible and avoid the typo. Ensure you
update the RUN lines in the Dockerfile where "RUN pip install uv" and "RUN uv
pip install --no-cache --system \"/app[all]\"" appear.
In `@pyproject.toml`:
- Line 28: The pyproject.toml change narrows dbt-core and adapter version ranges
(the dbt-core entry and the adapter entries referenced at lines 48-62) to
>=1.8,<2.0.0 which breaks users on dbt 1.7 and below; restore the original broad
ranges in pyproject.toml (keep dbt-core >=0.20 and adapters >=1.4 as per project
convention) and instead enforce the >=1.8 floor only in the Docker build by
adding a separate constraints file (e.g., constraints-dbt18.txt) and using pip's
--constraint during the Docker build process so pip/Poetry installs remain
backward-compatible.
…e version >=1.8) Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
Summary
The v0.23.0 Docker build fails with pip's
resolution-too-deeperror (failed job). pip 26.0.1 (installed viapip install --upgrade pip) includes a stricter resolution depth limit from resolvelib 1.1.0, and the[all]extra's 15 dbt adapters with wide version ranges (>=0.20) create a dependency graph too complex for the resolver.Three changes to fix this:
Dockerfile: Replace pip with uv for package installation, pinned to
0.10.11. uv's resolver handles complex dependency graphs without hitting depth limits. Both install steps are combined into a singleRUNlayer.pyproject.toml: Tighten lower bounds for
dbt-coreand all adapter extras from various old versions (>=0.20,>=1.4,>=1.5, etc.) to>=1.8. dbt versions before 1.8 are deprecated/EOL, and narrowing the version ranges reduces the resolver search space.pyproject.toml: Remove
dbt-fabricsparkfrom theallextra. There is no installable version ofdbt-fabricspark>=1.8— v1.9.0 has broken transitive deps (azure-keyvault-administration==4.4.0b2andazure-batch==15.0.0b1are pre-release pins that don't resolve), and v1.9.1 was yanked. The individualfabricsparkextra is retained but excluded from[all]until the upstream package is fixed.Updates since last revision
uv==0.10.11(the version verified in the successful local build) via aUV_VERSIONbuild arg for reproducibilityRUN pip install uv/RUN uv pip install ...steps into a single layer to reduce image sizedbt-fabricsparkfrom theallextra (broken upstream — no installable version >=1.8)Review & Testing Checklist for Human
dbt-fabricsparkremoval fromallis acceptable — the individual[fabricspark]extra still exists but will fail to install until the upstream package is fixed. Confirm this adapter can be temporarily excludedRecommended test plan: After merging, re-create the v0.23.0 tag and re-run the release workflow to verify the Docker image builds and pushes successfully on all target platforms.
Notes
python:3.10.7) ships with pip 22.2.2 which is used to install uv itself — this is fine since it's a single-package install with no complex resolution.ARG UV_VERSION=0.10.11so it can be bumped easily in future releases. The version can also be overridden at build time with--build-arg UV_VERSION=x.y.z.dbt-fabricsparkupstream issue: v1.9.0 depends onazure-cli>=2.60.0, which requires pre-release versions ofazure-keyvault-administrationandazure-batchthat don't resolve without--prerelease=allow. This is a packaging bug in dbt-fabricspark, not something we can fix here.Link to Devin session: https://app.devin.ai/sessions/a67f519882744e45b279502a0fe54bde
Requested by: @haritamar
Summary by CodeRabbit