Convert project to UV #599
Conversation
WalkthroughThe PR migrates CI and local tooling from Poetry/pipx to UV: GitHub Actions workflows now install/use UV, run Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
The errors in the CI would be due to the fact that we're upgrading mypy as part of this PR. I'd suggest that we also remove Python 3.9 with this shift to uv and that we merge it as soon as the SDK for Infrahub 1.5 is out. |
Deploying infrahub-sdk-python with
|
| Latest commit: |
e09e9e0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ce425c0c.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://dga-20251104-poetry-uv.infrahub-sdk-python.pages.dev |
github-actions
bot
added
the
type/documentation
dgarros
force-pushed
the
dga-20251104-poetry-uv
branch
2 times, most recently
from
f2c86fc to
2a2cf9b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## develop #599 +/- ##
===========================================
+ Coverage 75.46% 75.54% +0.08%
===========================================
Files 113 113
Lines 9512 9512
Branches 1893 1448 -445
===========================================
+ Hits 7178 7186 +8
- Misses 1834 1839 +5
+ Partials 500 487 -13
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 7 files with indirect coverage changes 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
148-151: Add explicit Python setup to documentation job.The documentation job (lines 148-151) runs
uv sync --all-groups --all-extraswithout an explicit Python setup step. While UV can discover system Python, this is implicit. For clarity and reliability, add an explicitactions/setup-python@v6step before theuv synccommand, similar to other jobs that use UV (e.g., lines 167-176).Apply this diff to add explicit Python setup:
- name: Install NodeJS uses: actions/setup-node@v5 with: node-version: 20 cache: 'npm' cache-dependency-path: docs/package-lock.json - name: "Install dependencies" run: npm install + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: "3.12" + - name: Install UV + uses: astral-sh/setup-uv@v4 - name: Install Python dependencies run: uv sync --all-groups --all-extras - name: "Build docs website" run: "uv run invoke docs"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
poetry.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.github/workflows/ci.yml(5 hunks).github/workflows/define-versions.yml(1 hunks).github/workflows/publish-pypi.yml(1 hunks).github/workflows/release.yml(2 hunks).gitignore(1 hunks)CLAUDE.md(3 hunks)README.md(1 hunks)docs/docs/python-sdk/guides/client.mdx(1 hunks)infrahub_sdk/ctl/cli.py(1 hunks)pyproject.toml(3 hunks)tasks.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.{md,mdx}: Follow the Diataxis framework and classify docs as Tutorials, How-to guides, Explanation, or Reference
Structure How-to guides with required frontmatter and sections: introduction, prerequisites, step-by-step steps, validation, related resources
Structure Topics (Explanation) docs with introduction, concepts & definitions, background & context, architecture & design, connections, further reading
Use a professional, concise, informative tone with consistent structure across documents
Use proper language tags on all code blocks
Include both async and sync examples where applicable using the Tabs component
Add validation steps to guides to confirm success and expected outputs
Use callouts for warnings, tips, and important notes
Define new terms on first use and use domain-relevant terminology consistent with Infrahub’s model/UI
Conform to markdown style rules in .markdownlint.yaml and Vale styles in .vale/styles/
Files:
docs/docs/python-sdk/guides/client.mdx
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
When implementing Infrahub checks, subclass InfrahubCheck and override validate(data); do not implement or rely on a check() method
Files:
tasks.pyinfrahub_sdk/ctl/cli.py
infrahub_sdk/ctl/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
infrahub_sdk/ctl/**/*.py: Build CLI commands with Typer
Organize and keep all CLI commands within infrahub_sdk/ctl/
Files:
infrahub_sdk/ctl/cli.py
🧠 Learnings (7)
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to infrahub_sdk/config.py : Environment variables for configuration must use the INFRAHUB_ prefix
Applied to files:
docs/docs/python-sdk/guides/client.mdxREADME.md
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to infrahub_sdk/config.py : Use a Pydantic-based Config class with environment variable support for configuration
Applied to files:
docs/docs/python-sdk/guides/client.mdxREADME.md
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to infrahub_sdk/client.py : Support authentication via API tokens or JWT with automatic refresh
Applied to files:
docs/docs/python-sdk/guides/client.mdx
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to tests/**/*.py : Use the custom pytest plugin (infrahub_sdk.pytest_plugin) fixtures for clients, configuration, and Infrahub-specific testing
Applied to files:
CLAUDE.mdREADME.md.github/workflows/ci.yml
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to tests/integration/**/*.py : Place and write integration tests under tests/integration/ (tests against real Infrahub instances)
Applied to files:
CLAUDE.md
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Build CLI commands with Typer
Applied to files:
tasks.pyinfrahub_sdk/ctl/cli.py
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Organize and keep all CLI commands within infrahub_sdk/ctl/
Applied to files:
tasks.pyinfrahub_sdk/ctl/cli.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.9)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (22)
.gitignore (1)
11-12: LGTM! Virtual environment exclusions added correctly.The addition of
.venv/and.venvpatterns appropriately excludes UV's default virtual environment directory.README.md (1)
59-73: LGTM! Clear UV setup instructions.The new development setup section provides helpful examples for installing dependencies with UV, covering both targeted and comprehensive installation scenarios.
docs/docs/python-sdk/guides/client.mdx (1)
20-20: LGTM! Installation instructions updated correctly.The prerequisite now correctly references UV for installing the SDK.
tasks.py (2)
42-47: LGTM! CLI documentation generation migrated to UV.The command correctly uses
uv run typerto generate documentation for individual CLI commands.
52-55: LGTM! Group documentation generation updated.The command properly invokes Typer documentation generation for command groups using UV.
infrahub_sdk/ctl/cli.py (1)
7-8: LGTM! Error message updated for UV workflow.The ImportError message now correctly instructs users to install the
ctlextra using either pip or UV.pyproject.toml (5)
1-31: LGTM! Successfully migrated to PEP 621 format.The project metadata has been correctly restructured from Poetry's format to the standard PEP 621 format, making it compatible with UV and other modern Python tooling.
38-42: LGTM! CLI and pytest plugin entry points defined correctly.The
infrahubctlCLI command and pytest plugin are properly exposed through standard entry points.
44-70: LGTM! Optional dependencies restructured appropriately.The optional dependencies for
ctlandallextras are correctly defined in PEP 621 format.
347-352: LGTM! Build system migrated to Hatchling.The build backend has been correctly changed from Poetry to Hatchling, with proper package specification for wheel building.
72-104: I'll verify the syntax through web search and codebase inspection sinceuvisn't available in the sandbox.I found references toinclude-groupbeing discussed. Let me search more specifically for PEP 735 include-group syntax to confirm whether it's actually supported in the standard.Based on my verification, the{include-group = "..."}syntax is part of PEP 735 specification and is supported by tools like PDM. UV's documentation for include-group support was pending at the time of the PEP 735 implementation, but the syntax itself is standard and valid according to the specification.The original review comment's concern about this syntax not being "standard" for UV's implementation is unfounded—it is part of the official PEP 735 specification that UV supports.
The
{include-group = "..."}syntax in pyproject.toml is valid and supported by UV.The syntax is part of PEP 735's official specification for dependency group includes. Your implementation correctly follows this standard. No changes are needed.
CLAUDE.md (3)
11-36: LGTM! Development commands migrated to UV.All essential development commands have been correctly updated to use UV instead of Poetry, providing clear instructions for developers.
42-50: LGTM! Testing commands updated consistently.Test execution commands correctly use
uv run pytestthroughout.
157-157: LGTM! Documentation generation command updated.The docs generation command correctly uses
uv run invoke docs..github/workflows/define-versions.yml (1)
7-8: Clarify UV version stability.The review comment's assertion that
0.9.8is outdated is incorrect. The latest stable uv release is v0.9.6 (released 2025-10-29), so the specified version0.9.8is actually newer. However, verify whether0.9.8is a pre-release or development version. If it is, consider whether using a pre-release in CI/CD is appropriate, or downgrade to the latest stablev0.9.6for production workflows.Likely an incorrect or invalid review comment.
.github/workflows/release.yml (3)
44-48: Verify tomllib compatibility across all Python versions in scope.tomllib was added in Python 3.11, but this workflow explicitly sets Python 3.12 (line 31), so it's safe here. However, if the project intends to support Python 3.9 or 3.10 anywhere, you'll need a fallback approach. Based on the PR objectives mentioning removal of Python 3.9 support, this usage appears intentional.
Confirm that Python 3.9 support has been formally dropped across all workflows and the project's declared supported versions have been updated in
pyproject.toml.
33-39: UV setup and dependency installation look good.The UV setup (line 33-36) correctly pulls the version from
prepare-environment, anduv sync --all-groups --all-extras(line 39) is the appropriate UV equivalent topoetry install. No concerns here.
56-64: Improved tag validation logic.The explicit tag version check (lines 56-64) is an improvement over implicit version matching. It clearly compares the expected tag derived from
pyproject.tomlagainst the actual release tag and provides useful error diagnostics on mismatch..github/workflows/publish-pypi.yml (1)
44-76: UV migration in publish workflow looks solid.The UV setup, dependency caching strategy, and command invocations all look correct. The cache key based on
uv.lock(line 59) is appropriate, the conditional install (line 63) is properly gated on cache hit, and theuv config token pypi(line 66),uv build(line 69), anduv publish(line 76) commands are the correct UV equivalents..github/workflows/ci.yml (3)
212-215: Remove Python 3.9 from test matrix if dropping support.uv supports Python 3.8, 3.9, 3.10, 3.11, and 3.12, so Python 3.9 is technically compatible with UV itself. However, the PR objectives mention "Suggests removing Python 3.9 with the shift to uv." If Python 3.9 support is being dropped, remove it from the test matrix (currently line 212). This should be consistent with the declared minimum Python version in
pyproject.toml.Confirm whether Python 3.9 support has been formally dropped, and if so, update the matrix accordingly. Also verify that
pyproject.tomlreflects the new minimum Python version.
232-244: UV integration in unit tests job looks good.The unit tests job correctly sets up UV (lines 232-235), installs dependencies (lines 236-237), and runs mypy (line 239), pytest (line 241), and codecov (line 244) via
uv run. The matrix includes multiple Python versions, and UV will manage Python discovery for each one.Ensure the Python 3.9 removal (if applicable) is reflected in the test matrix once finalized.
251-257: Coverage reporting via uv run is correct.Lines 251-257 correctly invoke coverage commands via
uv run. The multi-step coverage workflow (run, report, xml, codecov) is appropriate and the syntax is correct.
| documentation = "https://docs.infrahub.app/python-sdk/introduction" | ||
| packages = [{ include = "infrahub_sdk" }] | ||
| license = {text = "Apache-2.0"} | ||
| requires-python = ">=3.9,<3.14" |
There was a problem hiding this comment.
Resolve Python version inconsistency.
There's a mismatch between pyproject.toml (lines 10, 14: Python 3.9-3.13) and CLAUDE.md (line 177: "Multi-version Python testing (3.10-3.13)"). According to PR comments, ogenstad suggests "removing 3.9 with the shift to uv."
Consider either:
- Dropping Python 3.9 support entirely (update
requires-pythonto>=3.10,<3.14) - Updating
CLAUDE.mdto reflect 3.9 support if you intend to keep it
Based on PR comments by ogenstad.
Also applies to: 177-177
🤖 Prompt for AI Agents
In pyproject.toml around line 10 and CLAUDE.md line 177, the declared supported
Python versions are inconsistent and PR feedback recommends dropping 3.9; update
requires-python to ">=3.10,<3.14" in pyproject.toml and also change the
CLAUDE.md text at line 177 to "Multi-version Python testing (3.10-3.13)" so both
files reflect the same supported range.
| ] | ||
| lint = [ | ||
| "yamllint", | ||
| "mypy==1.11.2", |
There was a problem hiding this comment.
Address mypy upgrade causing CI failures.
According to PR comments, upgrading mypy to 1.11.2 is causing CI errors. Consider reverting to the previous mypy version or addressing the type-checking errors it surfaced.
Based on PR comments by ogenstad.
🤖 Prompt for AI Agents
In pyproject.toml at line 85, the mypy version was bumped to "mypy==1.11.2"
which is causing CI failures; either revert this line to the previous working
mypy version (restore the exact version from the last passing commit) or keep
1.11.2 and fix the new type errors: run mypy locally against the codebase,
address the reported type issues (or add targeted ignores/typing fixes), update
tests/CI config if needed, and re-run CI to confirm the failures are resolved.
4bc7eae to
70f7e13
Compare
dgarros
force-pushed
the
dga-20251104-poetry-uv
branch
from
70f7e13 to
70d0418
Compare
|
@dgarros, can you rebase this to fix the merge conflicts? That should also cause the failing tests to pass. |
dgarros
force-pushed
the
dga-20251104-poetry-uv
branch
from
70d0418 to
e6779d7
Compare
dgarros
force-pushed
the
dga-20251104-poetry-uv
branch
from
e6779d7 to
838fcf6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
pyproject.toml (2)
10-10: Resolve Python version inconsistency: remove Python 3.9 support.Per PR comments by ogenstad, Python 3.9 should be dropped with the shift to UV. Lines 10 and 14 currently declare Python 3.9 support, but this conflicts with the recommendation.
Update
requires-pythonto>=3.10,<3.14(line 10) and remove the Python 3.9 classifier (line 14) to align with the UV migration strategy. Verify that CLAUDE.md line 177 is also updated to reflect3.10-3.13testing range.Also applies to: 14-14
85-85: Address mypy upgrade causing CI failures.Per PR comments by ogenstad, mypy==1.11.2 is causing CI test failures. You must either revert to a known working version or fix the underlying type-checking errors that the newer mypy version uncovered.
Identify which mypy version was in use before this PR, revert line 85 to that version, or (if keeping 1.11.2) run
mypylocally to identify and resolve the new type errors, then re-run CI to confirm the failures are resolved.
🧹 Nitpick comments (1)
pyproject.toml (1)
58-70: Clarify intent and structure of "all" optional-dependencies group.The "all" optional-dependencies group (lines 58–70) conflates runtime optional features with test-only dependencies. Per PEP 735 best practices, test/dev dependencies should live in
[dependency-groups](e.g., "tests"), while[project.optional-dependencies]should contain runtime optional features (e.g., "ctl").Currently, pytest (line 63) appears in both
[project.optional-dependencies]["all"]and[dependency-groups]["tests"], creating duplication and semantic confusion. The "ctl" group is a clean runtime optional extra, but "all" breaks this pattern.Clarify the intent: Does "all" mean "all runtime optional features" (in which case, remove pytest and keep only ctl deps), or does it mean "everything for development" (in which case, it should not be an optional-dependency but rather a named profile in [dependency-groups])? If users install
pip install infrahub-sdk[all], should they get test dependencies, or only the optional runtime features like the CLI tools?If "all" is meant to group all runtime optional features for users, restructure as:
[project.optional-dependencies] ctl = [ "Jinja2>=3", "numpy>=1.24.2; python_version<'3.12'", "numpy>=1.26.2; python_version>='3.12'", "pyarrow>=14", "pyyaml>=6", "rich>=12,<14", "typer>=0.12.5", "click==8.1.*", "copier>=9.8.0", "ariadne-codegen==0.15.3", ] all = [ {include-group = "ctl"}, ]If test/dev dependencies were intended to be exposed via "all", move them to a named group in
[dependency-groups]instead, reserving[project.optional-dependencies]for runtime user-installable extras.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
pyproject.toml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
pyproject.toml (1)
347-352: Build system migration looks good.The transition from
poetry-coretohatchlingis correct. The[tool.hatch.build.targets.wheel]configuration (line 351) correctly specifies the package.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
pyproject.toml (2)
10-10: Resolve Python version inconsistency (duplicate of past review comment).Line 10 declares
requires-python = ">=3.9,<3.14"and lines 14–18 include Python 3.9 in classifiers. However, the past review flagged an inconsistency with CLAUDE.md (line 177: "Multi-version Python testing (3.10-3.13)"), and PR feedback from ogenstad recommends "removing 3.9 with the shift to uv." This issue remains unresolved.Decide whether to:
- Drop Python 3.9: Update line 10 to
requires-python = ">=3.10,<3.14"and remove the Python 3.9 classifier (line 14).- Keep Python 3.9: Update CLAUDE.md line 177 to document 3.9 support.
Whichever path you choose, ensure consistency across all files.
Also applies to: 14-18
85-85: Address mypy upgrade causing CI failures (duplicate of past review comment).Line 85 pins
mypy==1.11.2. The past review flagged that this version upgrade is causing CI errors. This issue remains unresolved.Either:
- Revert to the previous working mypy version (check the last passing commit).
- Keep 1.11.2 and fix the type-checking errors it surfaced: run mypy locally, address reported issues (or add targeted ignores), and re-run CI to confirm resolution.
🧹 Nitpick comments (1)
pyproject.toml (1)
45-56: Version pinning strategy is inconsistent across dependencies.Some dependencies use exact pins (e.g.,
click==8.1.*,ariadne-codegen==0.15.3,mypy==1.11.2,ruff==0.11.0), while others use version ranges (e.g.,Jinja2>=3,numpy>=1.24.2). This inconsistency makes it unclear whether the exact pins are intentional for stability/compatibility or oversight.Consider adopting a consistent pinning strategy:
- Exact pins for tools that frequently introduce breaking changes (linters, type-checkers: ruff, mypy).
- Ranges for libraries consumed by users (runtime dependencies, optional features).
This is particularly important for
ariadne-codegen==0.15.3andclick==8.1.*, which differ in precision from the rest.Also applies to: 83-88
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
pyproject.toml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.10)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
pyproject.toml (2)
29-29: Conditional marker for eval-type-backport depends on Python 3.9 removal decision.Line 29 specifies
eval-type-backport>=0.2.2; python_version>='3.9' and python_version<'3.10', which targets only Python 3.9. If Python 3.9 support is dropped (per prior feedback), this entire line becomes dead code and should be removed.
72-104: Dependency groups and build-system migration are well-structured.The migration to PEP 735
[dependency-groups](dev, tests, lint, types) for local/development-only contexts and[project.optional-dependencies](ctl, all) for runtime extras aligns with the stated 2025 best-practice recommendation in the PR objectives. The include-group syntax (lines 95–97) correctly composes dependencies.Build system transition to hatchling (lines 347–349) and wheel target configuration (lines 351–352) are correct.
Also applies to: 347-352
.github/workflows/release.yml
Outdated
| echo is_devrelease=$(poetry run python -c "from packaging.version import Version; print(int(Version('$(poetry version -s)').is_devrelease))") >> "$GITHUB_OUTPUT" | ||
| echo "version=$(poetry version -s)" >> "$GITHUB_OUTPUT" | ||
| echo major_minor_version=$(poetry run python -c "from packaging.version import Version; print(f\"{Version('$(poetry version -s)').major}.{Version('$(poetry version -s)').minor}\")") >> "$GITHUB_OUTPUT" | ||
| VERSION=$(uv run python -c "import tomllib; print(tomllib.load(open('pyproject.toml', 'rb'))['project']['version'])") |
There was a problem hiding this comment.
We should be able to use uv version --short here.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.9)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
.github/workflows/release.yml (2)
33-39: UV installation and dependency sync look appropriate.The migration from Poetry to UV setup and
uv sync --all-groups --all-extrasis correctly structured.
56-64: Tag validation logic is sound, but depends on correct VERSION extraction.The structure of constructing EXPECTED_TAG and comparing it to
github.event.release.tag_nameis correct. However, this validation will only work correctly once the VERSION variable (line 44) is fixed to return the actual project version, not the UV tool version.
| VERSION=$(uv version --short) | ||
| echo is_prerelease=$(uv run python -c "from packaging.version import Version; print(int(Version('$VERSION').is_prerelease))") >> "$GITHUB_OUTPUT" | ||
| echo is_devrelease=$(uv run python -c "from packaging.version import Version; print(int(Version('$VERSION').is_devrelease))") >> "$GITHUB_OUTPUT" | ||
| echo "version=$VERSION" >> "$GITHUB_OUTPUT" | ||
| echo major_minor_version=$(uv run python -c "from packaging.version import Version; v = Version('$VERSION'); print(f'{v.major}.{v.minor}')") >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🧩 Analysis chain
Critical: uv version --short returns the UV tool version, not the project version.
Line 44 uses uv version --short to extract the project VERSION, but this command returns the UV tool version (e.g., "0.4.28"), not the Python project version. This will:
- Break tag validation (line 58-59): the workflow will expect tag "v0.4.28" but the release tag is "v<project_version>"
- Invalidate prerelease/devrelease checks (lines 45-46) based on wrong version
- Cause the release workflow to fail
To fix, extract the project version from pyproject.toml or via Python. For example:
- VERSION=$(uv version --short)
+ VERSION=$(uv run python -c "from importlib.metadata import version; print(version('infrahub-sdk'))")Or, if the project defines __version__ in its package:
- VERSION=$(uv version --short)
+ VERSION=$(uv run python -c "import infrahub_sdk; print(infrahub_sdk.__version__)")Please verify the correct approach with the project's structure and confirm this resolves the version mismatch.
I need to verify the critical claim in this review comment about uv version --short. Let me inspect the repository structure and the workflow file.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check repository structure and find pyproject.toml
fd -t f "pyproject.toml|setup.py|setup.cfg|__init__.py" -x ls -la {} \;
Length of output: 2236
🏁 Script executed:
#!/bin/bash
# View the complete release.yml workflow for context
cat .github/workflows/release.ymlLength of output: 3677
🏁 Script executed:
#!/bin/bash
# Search for __version__ definitions in the codebase
rg -n "__version__" --type py -A2 -B2Length of output: 653
🏁 Script executed:
#!/bin/bash
# Check what uv version actually returns by examining documentation or similar workflows
rg -n "uv version" --type yml -C5Length of output: 27
Let me check the pyproject.toml to confirm the project name and verify the fix is correct.
---
🏁 Script executed:
```shell
#!/bin/bash
# Extract project name from pyproject.toml
head -50 pyproject.toml | grep -A 5 "\[project\]"
Length of output: 169
🏁 Script executed:
#!/bin/bash
# Check for any version field in pyproject.toml
rg -n "version\s*=" pyproject.toml | head -10Length of output: 20
Critical: uv version --short returns the UV tool version, not the project version.
Line 44 uses uv version --short to extract the project VERSION, but this command returns the UV tool version (e.g., "0.4.28"), not the Python project version ("1.15.0"). This breaks the workflow:
- Tag validation (lines 58-59) fails: the workflow expects tag "v0.4.28" but the release tag is "v1.15.0"
- Prerelease/devrelease checks (lines 45-46) evaluate the wrong version
- Release workflow fails with validation error
Fix by extracting the project version from package metadata:
- VERSION=$(uv version --short)
+ VERSION=$(uv run python -c "from importlib.metadata import version; print(version('infrahub-sdk'))")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| VERSION=$(uv version --short) | |
| echo is_prerelease=$(uv run python -c "from packaging.version import Version; print(int(Version('$VERSION').is_prerelease))") >> "$GITHUB_OUTPUT" | |
| echo is_devrelease=$(uv run python -c "from packaging.version import Version; print(int(Version('$VERSION').is_devrelease))") >> "$GITHUB_OUTPUT" | |
| echo "version=$VERSION" >> "$GITHUB_OUTPUT" | |
| echo major_minor_version=$(uv run python -c "from packaging.version import Version; v = Version('$VERSION'); print(f'{v.major}.{v.minor}')") >> "$GITHUB_OUTPUT" | |
| VERSION=$(uv run python -c "from importlib.metadata import version; print(version('infrahub-sdk'))") | |
| echo is_prerelease=$(uv run python -c "from packaging.version import Version; print(int(Version('$VERSION').is_prerelease))") >> "$GITHUB_OUTPUT" | |
| echo is_devrelease=$(uv run python -c "from packaging.version import Version; print(int(Version('$VERSION').is_devrelease))") >> "$GITHUB_OUTPUT" | |
| echo "version=$VERSION" >> "$GITHUB_OUTPUT" | |
| echo major_minor_version=$(uv run python -c "from packaging.version import Version; v = Version('$VERSION'); print(f'{v.major}.{v.minor}')") >> "$GITHUB_OUTPUT" |
This PR migrate the project to UV instead of poetry
For the dependencies in pyproject.toml, it's using a mix of [project.optional-dependencies] (PEP 621) and [dependency-groups] (PEP 735) as describe below
🧭 TL;DR Best practice (2025)
Use [dependency-groups] (PEP 735) for:
• Development dependencies (dev, test, bench)
• Local-only contexts (release, ci)
Use [project.optional-dependencies] (PEP 621) for:
• Runtime optional features (ctl)
• Plugin or extension systems
• Anything users install via pip install infrahub-sdk[ctk]
Summary by CodeRabbit
Chores
New Features
Documentation
Bug Fixes