chore: migrate from Poetry to uv for dependency management #283
Conversation
Replace Poetry with uv across all workflows, Dockerfile, and project configuration. This provides faster dependency resolution and a more modern Python packaging workflow. - Update GitHub Actions workflows to use astral-sh/setup-uv@v5 - Update Python version to 3.12 in trigger-push-stable workflow - Replace poetry.lock with uv.lock - Update Dockerfile to install uv and use uv sync - Simplify workflow matrices by removing Poetry version specifications
WalkthroughCI and Docker tooling migrated from Poetry to UV (installed via astral-sh/setup-uv@v7, uv v0.9.18) with Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
⏰ 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). (11)
🔇 Additional comments (2)
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 |
Deploying infrahub-ansible with
|
| Latest commit: |
a605a43
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a35f8005.infrahub-ansible.pages.dev |
| Branch Preview URL: | https://pmc-20251225-uv.infrahub-ansible.pages.dev |
![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 (3)
Dockerfile (1)
22-23: Pin the UV image version for reproducibility.Using the
:latesttag can lead to non-reproducible builds, as the image may change between builds. Pin to a specific version tag for consistency.🔎 Suggested fix
-# Install uv for dependency management -COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ +# Install uv for dependency management (pinned version) +COPY --from=ghcr.io/astral-sh/uv:0.5.15 /uv /uvx /bin/Note: Replace
0.5.15with the specific version you want to use.pyproject.toml (2)
9-14: Reconsidertoxas a main dependency.The
toxpackage is typically a development/testing tool rather than a runtime dependency. Consider moving it to the[dependency-groups] devsection unless there's a specific reason it needs to be installed in production environments.🔎 Proposed fix
dependencies = [ "ansible-core>=2.17.7", "infrahub-sdk[all]>=1.5,<2.0", "jinja2>=3.1.6", - "tox>=4.32.0", ] [dependency-groups] dev = [ + "tox>=4.32.0", "codecov>=2.1.13", "invoke>=2.2.1",
1-7: Consider standardizing the license format.While
{text = "GPLv3"}is valid, the SPDX identifier format is more standard:{text = "GPL-3.0-or-later"}or using thelicensefield with just the SPDX identifier.🔎 Suggested improvement
-license = {text = "GPLv3"} +license = {text = "GPL-3.0-or-later"}
📜 Review details
Configuration used: Organization 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 (5)
.github/workflows/trigger-push-stable.yml.github/workflows/workflow-changelog-and-docs.yml.github/workflows/workflow-linter.ymlDockerfilepyproject.toml
⏰ 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: ansible_linter_and_tests / sanity / sanity-py3.10-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.18
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.17
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
.github/workflows/workflow-linter.yml (1)
62-76: LGTM! Clean migration to UV-based linting.The workflow correctly:
- Sets up Python 3.12
- Installs UV using the official action
- Syncs dev dependencies with
uv sync --group dev- Runs linting commands via
uv runDockerfile (1)
28-29: The--no-install-projectflag is intentional and correct for this Ansible collection build.The
infrahub_ansible_modulesproject is an Ansible collection, not a traditional Python package requiring system installation. The pyproject.toml defines no entry points or console scripts, and the collection is built and installed exclusively viaansible-galaxy collection build(line 54) andansible-galaxy collection install(line 57). The project installation via pip is unnecessary in the Docker image since Ansible collections are deployed as .tar.gz archives rather than installed Python packages..github/workflows/workflow-changelog-and-docs.yml (1)
24-31: UV commands are syntactically correct.The
uv sync --group devcommand is valid and documented in the UV CLI reference. The syntax correctly specifies including the dev group in dependency synchronization, which is supported in UV 0.5.x.
Add explicit constraint for filelock>=3.20.1 to mitigate a security vulnerability in the transitive dependency pulled by tox>=4.32.0. Regenerated lockfile to ensure the patched version is used.
Update Hatchling build configuration to package all required files for ansible-galaxy collection install compatibility: - Add plugins/, roles/, meta/, docs/ directories to wheel packages - Include galaxy.yml, README.md, LICENSE, CHANGELOG.rst in both targets - Use force-include for root-level files in wheel build - Configure sdist to include pyproject.toml This ensures the built wheel and sdist contain all necessary Ansible collection components (plugins, roles, meta, docs, and manifest files).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pyproject.toml (1)
19-19: Remove redundant platform marker.The marker
python_version >= '3.10'is unnecessary sincerequires-python = ">=3.10,<3.14"(line 7) already ensures Python ≥3.10. All installations will satisfy this condition.🔎 Proposed simplification
- "numpy>=2.0.0; python_version >= '3.10'", + "numpy>=2.0.0",
📜 Review details
Configuration used: Organization 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
⏰ 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). (6)
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.20
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.10-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.11-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.17
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
pyproject.toml (1)
11-11: Excellent security fix for CVE-2025-68146.The explicit pin of
filelock>=3.20.1directly addresses the TOCTOU symlink/truncate vulnerability flagged in the previous review. This ensures the patched version is used despite it being a transitive dependency of tox.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/trigger-push-stable.yml (1)
26-26: Update commit message to include uv.lock.The commit message check looks for
"chore: update pyproject.toml & galaxy.yml", but the workflow now also commitsuv.lock(line 82). Consider updating the commit message (line 81) and this check to reflect the complete set of files being committed, e.g.,"chore: update pyproject.toml, galaxy.yml & uv.lock".🔎 Suggested fix
- if [[ "$commit_author" == "opsmill-bot" && \ - "$commit_message" == "chore: update pyproject.toml & galaxy.yml" ]]; then + if [[ "$commit_author" == "opsmill-bot" && \ + "$commit_message" == "chore: update pyproject.toml, galaxy.yml & uv.lock" ]]; thenAnd update line 81:
- commit-message: 'chore: update pyproject.toml & galaxy.yml' + commit-message: 'chore: update pyproject.toml, galaxy.yml & uv.lock'
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/trigger-push-stable.yml.github/workflows/workflow-changelog-and-docs.yml.github/workflows/workflow-linter.yml
⏰ 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). (8)
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.20
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.18
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.11-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.10-2.17
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
.github/workflows/trigger-push-stable.yml (2)
42-42: LGTM: Python 3.12 standardization.Simplifying to a single Python version (3.12) is appropriate for the uv migration and reduces CI complexity.
44-47: No action needed. The versions used are appropriate:astral-sh/setup-uv@v7correctly pins to the latest v7.x series (currently v7.1.6), and uv0.9.18is a valid, recent stable release from December 16, 2025..github/workflows/workflow-changelog-and-docs.yml (2)
22-27: LGTM: Consistent uv setup.The Python 3.12 and uv configuration matches the other workflows, maintaining consistency across the CI pipeline.
33-33: LGTM: Documentation generation command.The
uv run invoke generate-doccommand is correct for executing invoke tasks within the uv-managed environment, assuminginvokeis properly included in the dev dependencies..github/workflows/workflow-linter.yml (3)
62-70: LGTM: Consistent Python and uv setup.The Python 3.12 and uv configuration aligns with the other workflows in this PR, maintaining consistency.
76-78: LGTM: Ruff linting commands.The ruff commands are correctly formatted for execution via
uv run, assuming ruff is properly installed in the dev dependencies.
73-73: ruff is already included in thedevdependency group (ruff>=0.14.10), so theuv sync --group devcommand will correctly install all required linting dependencies.
pyproject.toml
Outdated
| requires-python = ">=3.10,<3.14" | ||
|
|
||
| dependencies = [ | ||
| "ansible-core>=2.17.7", |
There was a problem hiding this comment.
Not sure if this is on purpose but for Python 3.10 the version of ansible-core was ">=2.15.13rc1"
There was a problem hiding this comment.
It was on purpose, updated to the latest bugfix version that supports Python 3.10
pyproject.toml
Outdated
| numpy = [ | ||
| { version = "^1.24.2", optional = true, python = ">=3.9,<3.12" }, | ||
| { version = "^1.26.2", optional = true, python = ">=3.12" }, | ||
| "numpy>=2.0.0; python_version >= '3.10'", |
There was a problem hiding this comment.
Same here, the version of numpy is different
There was a problem hiding this comment.
It was on purpose, updated to the latest bugfix version that supports Python 3.10
pyproject.toml
Outdated
| build-backend = "hatchling.build" | ||
|
|
||
| [tool.hatch.build] | ||
| include = [ |
There was a problem hiding this comment.
this looks surprising, it feels like we are listing what should be included 3 times
The previous hatchling configuration included docs/ with node_modules and webpack build artifacts, bloating the wheel to 60MB. This change: - Remove docs/ from wheel (documentation should be accessed online) - Use only-include instead of packages directive for non-Python dirs - Move tox from dependencies to dev dependency group - Update ansible-core to >=2.17.14 and numpy to >=2.2.6 - Add pytest-pythonpath to dev dependencies
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization 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
⏰ 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). (13)
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-milestone
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.20
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-milestone
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.20
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.18
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.11-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.18
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.11-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.10-2.17
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
pyproject.toml (7)
1-8: Clean PEP 621 migration.The project metadata has been correctly migrated from Poetry format to PEP 621 standard. The structure, authors format, license declaration, and Python version constraints are all properly specified.
9-14: Dependencies include required security patches.All dependency versions have been updated to include critical security fixes (as verified in previous review rounds):
- ansible-core 2.17.14 (CVE-2024-11079, CVE-2024-8775)
- jinja2 3.1.6 (CVE-2025-27516)
- filelock 3.20.1 (CVE-2025-68146, appropriately documented inline)
16-19: Simplified numpy dependency specification.The consolidation of numpy requirements into a single version constraint with an environment marker is cleaner and more maintainable than the previous two-range approach.
21-33: Dev dependencies properly migrated.The dev dependency group contains all required tooling and test dependencies. The restoration of
pytest-pythonpath(per previous feedback) ensures test environment compatibility.
152-155: Build backend successfully migrated to Hatchling.The build system declaration correctly specifies Hatchling as the build backend, completing the migration away from Poetry.
156-166: Appropriate build exclusions.The global exclusions properly remove non-essential directories (tests, CI config, build artifacts) from distributions. The documented rationale for excluding
docs/(contains node_modules and build artifacts) is sound.
168-178: Source distribution properly configured.The sdist target correctly includes all Ansible collection directories (plugins, roles, meta) and essential metadata files (galaxy.yml, README.md, LICENSE, CHANGELOG.rst, pyproject.toml). The use of glob patterns ensures complete directory contents are captured.
| [tool.hatch.build.targets.wheel] | ||
| # For Ansible collections distributed as wheels, we need to include the | ||
| # collection structure (plugins, roles, meta) as data files, not Python packages. | ||
| # Using only-include ensures we get exactly what we need without bloat. | ||
| only-include = [ | ||
| "plugins", | ||
| "roles", | ||
| "meta", | ||
| "galaxy.yml", | ||
| "README.md", | ||
| "LICENSE", | ||
| "CHANGELOG.rst", | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Build both sdist and wheel, then test installation with ansible-galaxy
set -e
echo "=== Building distributions with uv ==="
uv build
echo -e "\n=== Listing built distributions ==="
ls -lh dist/
echo -e "\n=== Inspecting wheel contents ==="
python3 -m zipfile -l dist/*.whl | head -30
echo -e "\n=== Checking if ansible-galaxy supports wheel format ==="
# Try to install from wheel
if command -v ansible-galaxy &> /dev/null; then
echo "Testing ansible-galaxy collection install from wheel..."
ansible-galaxy collection install dist/*.whl --force 2>&1 || echo "Wheel installation failed or unsupported"
echo -e "\n=== Testing ansible-galaxy collection install from tarball ==="
ansible-galaxy collection install dist/*.tar.gz --force 2>&1 || echo "Tarball installation also failed"
else
echo "ansible-galaxy not available in sandbox"
fiRepository: opsmill/infrahub-ansible
Length of output: 154
🌐 Web query:
ansible-galaxy collection install wheel format support
💡 Result:
Short answer: No — ansible-galaxy does not support installing collections from Python wheel (.whl) files. Collections are distributed as collection artifacts (tar.gz built with ansible-galaxy collection build) or installed from Galaxy/git/url/path; Ansible wheels refer to the Ansible Python package itself, not collection artifacts. [1][2]
Sources:
- Ansible docs — installing collections (tarball/path/git/galaxy). [1]
- Discussion on Galaxy packaging/options (collections vs Python wheels). [2]
The wheel target is unnecessary for Ansible collection distribution.
ansible-galaxy collection install does not support wheel format—collections must be distributed as tar.gz archives (from sdist). The wheel configuration shown here will not be used for Ansible collection installation. While having the wheel target won't cause issues (sdist remains the primary distribution format), you can remove the [tool.hatch.build.targets.wheel] section if wheels aren't intended for other purposes.
If wheels are needed for non-collection use cases, keep the configuration; otherwise, consider removing it to reduce distribution complexity.
🤖 Prompt for AI Agents
In pyproject.toml around lines 180 to 192, the [tool.hatch.build.targets.wheel]
section is unnecessary for Ansible collection distribution because
ansible-galaxy installs collections from sdist (tar.gz) not wheels; remove the
entire wheel target block (lines 180–192) to avoid accidental distribution
complexity, or if you actually need wheels for other use cases, add a comment
explaining that it's intentionally present and keep the configuration as-is.
Add extensive Vale style exceptions for documentation linting: - Add sentence-case exceptions for headings (VSCode, OAuth, YAML, etc.) - Add spelling exceptions for technical terms and project-specific words
- Add CODEOWNERS file for repository ownership - Update workflow-linter.yml to use uv for dependency management - Upgrade Vale to v3.13.0 and paths-filter to opsmill/[email protected] - Add mypy and yamllint to dev dependencies with configuration - Sort dev dependencies alphabetically in pyproject.toml
Add mypy to CI workflow and fix all type errors across the codebase: Bug fixes: - Fix typo `orde=order` → `order=order` in fetch_artifacts - Fix `results = list[result]` → `results: list[...] = []` - Fix `node.value` → `node.content_type.value` attribute access - Fix `self.default_branch` → `self.client.config.default_branch` Type annotation fixes: - Add return type annotations to all public functions - Fix validate_certs type from str to bool - Add proper dict[str, Any] annotations for mixed-type dicts - Fix ImportError | None variable declarations Configuration: - Add types-PyYAML dev dependency - Configure mypy to exclude complex infrahub_utils module - Add pyright config to disable redundant checks - Add mypy step to workflow-linter.yml
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/lookup/lookup.py (1)
159-160: Incorrect error message: validation checks for dict, but message says "list of dict".The validation at line 159 checks if
graph_variablesis adict, but the error message incorrectly states it "must be a list of dict". The message should read "must be a dict" or "must be a dictionary".🔎 Proposed fix
if graph_variables is not None and not isinstance(graph_variables, dict): - raise AnsibleLookupError("graph_variables parameter must be a list of dict") + raise AnsibleLookupError("graph_variables parameter must be a dict")
🧹 Nitpick comments (3)
plugins/lookup/lookup.py (1)
132-132: Documentation correctly reflects list return type.The docstring now accurately describes the return type as
listinstead of the ambiguous type mentioned in the RETURN documentation block (which still saysdictat line 94).Consider updating the module-level
RETURNdocumentation (line 91-95) to reflect that the return type is actually alist, not adict, to maintain consistency with the actual implementation and the updated docstring..vale/styles/Infrahub/sentence-case.yml (2)
14-272: Consider sorting the exceptions list alphabetically.The exceptions list would be easier to maintain and review if sorted alphabetically. This helps quickly locate entries, identify duplicates, and keep the list organized as it grows.
💡 Script to generate a sorted version
#!/bin/bash # Extract, sort, and display the exceptions list alphabetically echo "Sorted exceptions list:" gawk ' /^ - / { gsub(/^ - /, "") print $0 } ' .vale/styles/Infrahub/sentence-case.yml | sort -f | gawk '{print " - " $0}'
14-272: Remove redundant case-insensitive duplicate entries to improve maintainability.The exceptions list contains 19 pairs of near-duplicate entries that differ only in capitalization:
- How-to Guides / How-to guides
- Intelligent YAML Editing / Intelligent YAML editing
- Key Features at a Glance / Key features at a glance
- Multi-Server Management / Multi-server management
- NetBox / Netbox
- Netpicker / NetPicker
- Next Steps / Next steps
- Quick Start / Quick start
- Real-time Status / Real-time status
- Troubleshooting Tips / Troubleshooting tips
- Understanding How It Works / Understanding how it works
- Use Cases / Use cases
- Version Information / Version information
- Visual Tree Views / Visual tree views
- VSCode Extension API / VSCode extension API
- What You'll Learn / What you'll learn
- Within the Extension / Within the extension
- Working with Branches / Working with branches
- Working with Multiple Environments / Working with multiple environments
Since Vale performs exact string matching on exceptions, consolidating to a single capitalization per entry would reduce list size without affecting functionality, unless your documentation genuinely uses both capitalization styles for the same heading.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.github/CODEOWNERS.github/workflows/workflow-linter.yml.vale/styles/Infrahub/sentence-case.yml.vale/styles/spelling-exceptions.txtplugins/action/artifact_fetch.pyplugins/action/query_graphql.pyplugins/inventory/inventory.pyplugins/lookup/lookup.pyplugins/module_utils/branch.pyplugins/module_utils/exception.pyplugins/module_utils/infrahub_utils.pyplugins/module_utils/node.pyplugins/modules/artifact_fetch.pyplugins/modules/branch.pyplugins/modules/node.pyplugins/modules/query_graphql.pypyproject.tomltasks/__init__.pytasks/docs.pytasks/utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/workflow-linter.yml
🧰 Additional context used
🧬 Code graph analysis (6)
plugins/modules/query_graphql.py (4)
plugins/inventory/inventory.py (1)
main(283-319)plugins/modules/artifact_fetch.py (1)
main(104-129)plugins/modules/branch.py (1)
main(104-118)plugins/modules/node.py (1)
main(106-120)
plugins/modules/branch.py (3)
plugins/modules/artifact_fetch.py (1)
main(104-129)plugins/modules/node.py (1)
main(106-120)plugins/modules/query_graphql.py (1)
main(113-131)
plugins/module_utils/node.py (5)
plugins/action/artifact_fetch.py (1)
run(29-117)plugins/action/query_graphql.py (1)
run(29-105)plugins/lookup/lookup.py (1)
run(117-183)plugins/module_utils/branch.py (1)
run(19-37)plugins/module_utils/infrahub_utils.py (1)
run(1476-1480)
plugins/action/artifact_fetch.py (1)
plugins/action/query_graphql.py (1)
run(29-105)
plugins/inventory/inventory.py (4)
plugins/modules/artifact_fetch.py (1)
main(104-129)plugins/modules/branch.py (1)
main(104-118)plugins/modules/node.py (1)
main(106-120)plugins/modules/query_graphql.py (1)
main(113-131)
plugins/module_utils/branch.py (1)
plugins/module_utils/infrahub_utils.py (1)
_handle_errors(1009-1016)
🪛 LanguageTool
.vale/styles/spelling-exceptions.txt
[style] ~155-~155: Un mot est répété.
Context: ...lTreeViewProvider Instruqt IP IPAddress IPAM Ipam IPHost IPPrefix IPs is_empty is_ip_with...
(FRENCH_WORD_REPEAT_RULE)
[style] ~199-~199: Un mot est répété.
Context: ...espace namespaces Nautobot Neo4j Netbox Netpicker NetPicker Netutils Newsfragment Nornir npm Number...
(FRENCH_WORD_REPEAT_RULE)
[style] ~231-~231: Un mot est répété.
Context: ...opulate_security_relationships Postgres PostgreSQL Postgresql Postgress Prefetch prefilled profile_na...
(FRENCH_WORD_REPEAT_RULE)
⏰ 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). (9)
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.20
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.11-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-milestone
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.11-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.10-2.17
- GitHub Check: Cloudflare Pages
🔇 Additional comments (32)
.github/CODEOWNERS (1)
1-3: LGTM!The CODEOWNERS file is well-formed with clear documentation and assigns default ownership to the team. If the need arises for more granular per-module or per-team ownership rules in the future, additional patterns can be added above the wildcard catch-all.
.vale/styles/spelling-exceptions.txt (1)
3-338: Comprehensive spelling exceptions additions align with PR objectives.The additions appropriately expand the Vale spelling exceptions to cover new tooling (
uv,certifi), infrastructure/networking terms (ASN,IPAM,FQDN,Anycast,colocation), authentication/protocols (OAuth2,OIDC,OpenID Connect), and monitoring platforms (Grafana,LibreNMS). The alphabetical ordering is maintained throughout, and all changes are purely additive.Note: Static analysis flagged three entries as repeated words (LanguageTool), but these are false positives—they correctly capture case-variant and distinct but related terms necessary for comprehensive spelling exception coverage (e.g.,
Postgres,PostgreSQL,Postgresql,Postgress).tasks/docs.py (1)
295-295: LGTM: Good defensive type-safety improvement.The None-check prevents potential
AttributeErrorwhen accessingoutput.exitedand aligns with the mypy type checking improvements across the codebase.plugins/module_utils/node.py (1)
16-16: LGTM: Correct return type annotation.The
-> Nonereturn type annotation is correct, as the method terminates viaself.module.exit_json()and doesn't return a value. This aligns with the broader type-hint improvements across the codebase.plugins/modules/branch.py (1)
104-104: LGTM: Consistent return type annotation.The
-> Nonereturn type annotation is correct and consistent with other module entry points (plugins/modules/node.py,plugins/modules/artifact_fetch.py,plugins/modules/query_graphql.py).plugins/modules/artifact_fetch.py (1)
104-104: LGTM: Correct return type annotation.The
-> Nonereturn type annotation is accurate, as the function only instantiates anAnsibleModuleand doesn't return a value. This is consistent with the type-hint pattern applied across other module entry points.plugins/modules/query_graphql.py (1)
113-113: LGTM: Consistent return type annotation.The
-> Nonereturn type annotation is correct and aligns with the broader pattern of explicit type hints for module entry points across the codebase.plugins/modules/node.py (1)
106-106: LGTM: Correct return type annotation.The
-> Nonereturn type annotation is accurate and consistent with the pattern applied to other module entry points in this PR.plugins/module_utils/branch.py (4)
8-9: LGTM: Necessary import for type annotations.The
Anyimport fromtypingis correctly added to support the type annotations introduced in this file.
19-19: LGTM: Correct return type annotation.The
-> Nonereturn type annotation is accurate, as the method terminates viaself.module.exit_json()without returning a value. This is consistent with similar methods across the codebase (e.g.,plugins/module_utils/node.py).
26-26: LGTM: Helpful variable type annotation.The explicit type annotation
dict[str, Any]forself.resultimproves type safety and helps static type checkers verify correct usage throughout the method.
35-35: LGTM: Cleaner direct assignment.Refactoring from
self.result.update({"branch": serialized_object})toself.result["branch"] = serialized_objectis functionally equivalent and more direct for single-key updates.plugins/action/query_graphql.py (2)
29-29: LGTM: More specific return type annotation.The return type
dict[str, Any]is more specific and helpful for type checking than the genericdicttype. This aligns with the broader type-hint improvements in the PR.
47-48: LGTM: Correct return behavior for skipped tasks.The explicit
return resultwhen the task is skipped is correct and consistent with the same pattern inplugins/action/artifact_fetch.py(lines 47-48 in the relevant snippets). This ensures the method always returns adict[str, Any]as declared, rather than implicitly returningNone.plugins/module_utils/exception.py (1)
15-16: Clean initialization pattern for optional import error tracking.Pre-initializing
INFRAHUBCLIENT_IMPORT_ERRORat module scope before the try/except block is a cleaner approach than the previous else-branch pattern. The type annotationImportError | Nonecorrectly reflects the possible states.plugins/module_utils/infrahub_utils.py (4)
79-79: Type annotation correctly aligned with argument spec.The
validate_certsparameter type is nowbool | None, which is consistent with theINFRAHUB_ARG_SPECdefinition at line 47 wherevalidate_certsis declared astype="bool".
160-178: Good fix: per-node result initialization prevents shared-state bug.Moving the
resultdictionary initialization inside the loop (lines 169-172) is correct. If defined outside the loop and reused, all appended items would reference the same dictionary object, causing incorrect results. The explicit type annotationdict[str, Any]also improves code clarity.
264-264: Proper type annotation for nodes list.Initializing
nodeswith an explicit type annotationlist[InfrahubNodeSync]improves type safety and IDE support.
329-329: Correct access to default branch via client config.Using
self.client.config.default_branchis the proper way to access the configured default branch from the underlyingInfrahubClientSyncinstance.plugins/action/artifact_fetch.py (2)
29-29: Return type annotation added for consistency.The explicit
dict[str, Any]return type aligns with the pattern used inquery_graphql.py(line 28) and improves type safety.
47-48: Correct fix: return result dict instead of None when skipped.Returning
result(which contains skip information from the parent'srun()method) is the correct behavior. This aligns with the pattern inquery_graphql.py(lines 45-46) where the result is returned when skipped. ReturningNonepreviously would have been inconsistent with the declared return type and could cause issues downstream.plugins/lookup/lookup.py (2)
122-124: Type annotations improve clarity and correctness.The explicit type hints for
graph_variables,kwargs, and return typelist[Any]align with the actual implementation and improve IDE support.
137-138: Appropriate type annotations for optional string values.The
str | Noneannotations correctly express that these values may beNoneif not provided via kwargs or environment variables.plugins/inventory/inventory.py (2)
167-168: Consistent module-level import error tracking pattern.The
PACKAGING_IMPORT_ERRORinitialization follows the same pattern established inplugins/module_utils/exception.pyforINFRAHUBCLIENT_IMPORT_ERROR, providing consistent error tracking across the codebase.
195-195: Return type annotations added for method signatures.The explicit
-> Nonereturn type annotations for_set_authorization,_store_in_cache,set_hosts_and_groups,set_host_variables,main, andparsemethods improve type safety and are consistent with similar patterns across other modules (e.g.,main() -> Noneinnode.py,branch.py).Also applies to: 231-231, 243-243, 269-269, 283-283, 321-321
tasks/__init__.py (1)
17-20: The# type: ignore[arg-type]comments are correct and necessary.Invoke's
add_collection()method accepts task-containing modules at runtime despite its type signature requiringCollectionobjects. This is idiomatic invoke usage—wrapping modules in explicitCollection()objects would be non-standard and unnecessary. The project uses invoke 2.2.1, which already includes full type annotations (available since 2.1.2), and there is no updated stub package or alternative typing approach that would eliminate these suppressions. The type ignores are the appropriate solution for this known limitation in invoke's type annotations.Likely an incorrect or invalid review comment.
tasks/utils.py (1)
8-21: LGTM! Type-safe refactor aligns with mypy adoption.The explicit
dict[int, str]construction withord()calls is functionally equivalent to the previousstr.maketrans()approach while providing clearer type hints for static analysis. This change supports the broader mypy integration introduced in this PR.pyproject.toml (5)
21-36: Dev dependencies properly migrated.The transition from
[tool.poetry.group.dev.dependencies]to[dependency-groups]is correct. All necessary development tools are included, includingpytest-pythonpath(line 30) which addresses a previous review concern.
45-71: Type checking configuration appropriately balances strictness with pragmatism.The mypy configuration enables strict checking (
disallow_untyped_defs = true) while providing necessary escape hatches:
- Excludes
infrahub_utils.pyfor complex exception handling patterns- Relaxes checks for
tasks.*to accommodate Invoke's typing constraints- Disables pyright in favor of mypy to avoid conflicting type checker behavior
This aligns well with the PR's goal of introducing static type checking incrementally.
73-182: Comprehensive Ruff configuration with pragmatic ignore lists.The extensive ignore list is well-documented, distinguishing between rules under investigation and rules that need code updates. Per-file ignores for Ansible-specific directories (
plugins/,tasks/) appropriately account for Ansible module conventions that differ from standard Python practices.
183-223: Build system migration to Hatchling is well-configured.The Hatchling configuration properly packages the Ansible collection structure:
sdist target (lines 199-209): Correctly includes all required collection artifacts (
galaxy.yml,README.md,plugins/,roles/,meta/) foransible-galaxy collection installfrom tarball.wheel target (lines 211-223): Uses
only-includeinstead ofpackages, which correctly treats the collection structure as data files rather than Python packages. This addresses the concern from previous reviews.Note on wheel target: As highlighted in past review comments,
ansible-galaxydoes not support installing collections from wheel format—collections must be distributed as tar.gz (sdist). The wheel configuration is technically sound but unnecessary for Ansible distribution. If wheels aren't needed for other use cases, consider removing lines 211-223 to simplify the build configuration. Otherwise, the current setup is harmless and correctly structured.
1-19: Migration to PEP 621 metadata format is correct.The project metadata and dependency specifications are properly structured. The security-conscious pinning of
filelock>=3.20.1addresses CVE-2025-68146 as intended. The specification ofansible-core>=2.17.14is valid—this version is the latest available in the 2.17.x series and aligns with the Python 3.10-3.13 support matrix.
- Add pytest-asyncio to dev dependencies - Add upper bound to invoke dependency (<3.0.0) - Enable stricter mypy settings (warn_return_any, warn_unused_ignores) - Add explicit permissions (contents: read) to workflow files - Rename LICENSE to LICENSE.txt for consistency - Rename .markdownlint.yml to .markdownlint.yaml for consistency - Update pyproject.toml references to LICENSE.txt
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
pyproject.toml (1)
212-224: Reconsider the wheel target for Ansible collections.A past review comment correctly noted that
ansible-galaxy collection installdoes not support wheel format—Ansible collections must be distributed as tar.gz archives (from sdist). The comment on lines 213-215 claims wheels can be used for Ansible collections, but this contradicts Ansible's documentation and tooling.While having the wheel configuration won't break the build, it adds unnecessary complexity. If wheels serve a different purpose (e.g., for non-Ansible use cases), please clarify this in the comment. Otherwise, consider removing the wheel target configuration to simplify the build process.
Based on previous review comments indicating that ansible-galaxy does not support wheel installation.
🧹 Nitpick comments (1)
.github/workflows/workflow-linter.yml (1)
71-71: Consider testing across the supported Python version range.The workflow now hardcodes Python 3.12, but
pyproject.tomldeclares support for Python 3.10-3.13 (requires-python = ">=3.10,<3.14"). While using a single version simplifies the workflow, consider whether you want to test against the minimum and maximum supported versions to catch compatibility issues.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/workflows/trigger-pr-develop.yml.github/workflows/trigger-pr-stable.yml.github/workflows/workflow-ansible-linter-and-tests.yml.github/workflows/workflow-linter.yml.markdownlint.yamlLICENSE.txtpyproject.toml
⏰ 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). (5)
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.11-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.10-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.17
- GitHub Check: Cloudflare Pages
🔇 Additional comments (13)
.github/workflows/trigger-pr-develop.yml (1)
13-14: Security improvement: explicit read-only permissions.Adding
permissions: contents: readfollows GitHub Actions security best practices by explicitly restricting the default GITHUB_TOKEN to read-only access, implementing the principle of least privilege..github/workflows/workflow-ansible-linter-and-tests.yml (1)
28-29: Security improvement: explicit read-only permissions.Adding
permissions: contents: readconsistently applies the principle of least privilege across the workflow suite..github/workflows/trigger-pr-stable.yml (2)
10-11: Security improvement: explicit read-only permissions.Consistently applying
permissions: contents: readacross all PR workflows strengthens the repository's security posture.
17-18: Good alignment with develop workflow.Adding the
ansible_linter_and_testsjob brings the stable workflow into consistency with the develop workflow, ensuring both branches receive the same testing coverage..github/workflows/workflow-linter.yml (2)
17-18: Good security practice.Adding explicit read-only permissions follows the principle of least privilege and aligns with GitHub's security recommendations.
85-86: Excellent addition of mypy type checking.Adding the mypy step strengthens type safety and aligns with the stricter mypy configuration added to
pyproject.toml.pyproject.toml (7)
1-8: LGTM: Clean PEP 621 migration.The project metadata has been properly migrated to PEP 621 format with correct authors, license, and Python version constraints.
9-14: Dependencies properly secured.The dependency specifications include appropriate security patches, particularly the
filelock>=3.20.1pin to address CVE-2025-68146. The inline comment clearly documents the security rationale.
16-37: LGTM: Dependency groups properly configured.The optional dependencies and dev dependency group are well-structured. The invoke version cap (
<3.0.0) and pytest-pythonpath inclusion align with the PR objectives.
46-73: Well-configured type checking setup.The mypy configuration appropriately balances strictness with pragmatism:
- Stricter settings (
warn_return_any,warn_unused_ignores) improve type safety- Targeted exclusions and overrides are well-documented
- Disabling pyright in favor of mypy is a reasonable choice to avoid tool conflicts
The inline comments clearly explain the rationale for each configuration decision.
184-186: LGTM: Standard Hatch build backend.The migration to Hatch with hatchling as the build backend is correctly configured.
188-198: LGTM: Appropriate exclusions for distribution.The exclusion patterns correctly prevent unnecessary files (docs, tests, CI configs) from being included in distribution packages.
200-210: LGTM: Comprehensive sdist configuration.The sdist target properly includes all necessary Ansible collection files (
galaxy.yml, collection directories, documentation, and license files).
| uses: "actions/checkout@v6" | ||
| - name: Check for file changes | ||
| uses: dorny/paths-filter@v3 | ||
| uses: opsmill/paths-filter@v3.0.2 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the current state of both repositories and the usage
echo "=== Checking opsmill/paths-filter repository ==="
curl -s "https://api.github.com/repos/opsmill/paths-filter" | jq -r '.name, .description, .pushed_at, .archived, .fork, .forks_count, .stargazers_count' 2>/dev/null || echo "Could not fetch opsmill/paths-filter"
echo -e "\n=== Checking original dorny/paths-filter repository ==="
curl -s "https://api.github.com/repos/dorny/paths-filter" | jq -r '.name, .description, .pushed_at, .archived, .forks_count, .stargazers_count' 2>/dev/null || echo "Could not fetch dorny/paths-filter"
echo -e "\n=== Search for opsmill/paths-filter usage in workflows ==="
rg "opsmill/paths-filter" .github/workflows/ --no-heading -n 2>/dev/null || echo "No matches found"
echo -e "\n=== Search for dorny/paths-filter usage in workflows ==="
rg "dorny/paths-filter" .github/workflows/ --no-heading -n 2>/dev/null || echo "No matches found"
echo -e "\n=== Check if this is the only instance ==="
find .github/workflows -name "*.yml" -o -name "*.yaml" | wc -lRepository: opsmill/infrahub-ansible
Length of output: 716
Switch back to the official dorny/paths-filter action.
The codebase uses opsmill/[email protected] (which is a fork), but the official dorny/paths-filter is more actively maintained (last updated November 2025 vs November 2024 for the fork) and has significantly broader community adoption (2,845 stars, 346 forks vs 0 for the fork). There's no apparent justification in the code for using the fork. Unless specific features or fixes from the fork are required, switch to dorny/paths-filter@v3 to reduce maintenance burden and ensure access to the latest updates.
🤖 Prompt for AI Agents
.github/workflows/workflow-linter.yml around line 36: the workflow currently
uses the fork opsmill/[email protected]; change it to the official action
dorny/paths-filter@v3 by replacing the uses entry with uses:
dorny/paths-filter@v3 to adopt the maintained upstream action and reduce
maintenance burden.
| - name: "Install uv" | ||
| uses: "astral-sh/setup-uv@v7" | ||
| with: | ||
| version: "0.9.18" | ||
| - name: "Install dependencies" | ||
| run: "uv sync --group dev" | ||
| - name: "Linting: yamllint" | ||
| run: "yamllint ." | ||
| run: "uv run yamllint -s ." |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/workflow-linter.yml | sed -n '48,78p'Repository: opsmill/infrahub-ansible
Length of output: 1113
🌐 Web query:
uv latest version December 2025 astral-sh uv releases
💡 Result:
The latest astral-sh/uv release is 0.9.17 — released December 9, 2025. [1][2]
Sources:
- GitHub Releases: astral-sh/uv (release 0.9.17, Dec 9, 2025). [1]
- GitHub Packages (uv images/tags showing 0.9.17). [2]
Update uv version to match latest released version.
The workflow pins uv to version 0.9.18, but the latest released version is 0.9.17 (December 9, 2025). Version 0.9.18 either does not exist or is not publicly released. Update to 0.9.17 or the intended version.
Also applies to: 73-76
🤖 Prompt for AI Agents
.github/workflows/workflow-linter.yml lines 50-57 (and similarly lines 73-76):
the workflow pins the astral-sh/setup-uv action to version "v7" with the uv tool
version set to "0.9.18", but that release does not exist; change the uv version
string to the latest published release "0.9.17" (or the intended released
version) in both locations so the action uses a valid uv release and the
workflow will run successfully.
- Update galaxy.yml to reference LICENSE.txt - Add explicit type annotations to fix mypy warn_return_any errors in query_graphql.py, artifact_fetch.py, and tasks/docs.py
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugins/action/query_graphql.py (1)
44-44: LGTM: Explicit type annotation added.The type annotation helps mypy infer types correctly. The
# noqa: UP008suppresses a style warning about old-stylesuper()syntax—modern Python allowssuper().run(tmp, task_vars)without explicit class arguments, though this is purely stylistic.🔎 Optional: Modernize super() call
If you'd like to address the UP008 warning instead of suppressing it:
- result: dict[str, Any] = super(ActionModule, self).run(tmp, task_vars) # noqa: UP008 + result: dict[str, Any] = super().run(tmp, task_vars)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
galaxy.ymlplugins/action/artifact_fetch.pyplugins/action/query_graphql.pytasks/docs.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tasks/docs.py
- plugins/action/artifact_fetch.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). (12)
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-milestone
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.18
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.13-2.20
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.11-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.11-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.10-2.17
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.19
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-milestone
- GitHub Check: ansible_linter_and_tests / sanity / sanity-py3.12-2.18
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
galaxy.yml (1)
32-32: LICENSE.txt exists in the repository root. The license file reference update from "LICENSE" to "LICENSE.txt" is correct and the referenced file exists, so this change will not cause Ansible Galaxy collection builds to fail.plugins/action/query_graphql.py (2)
29-29: LGTM: Type annotation improves clarity.The explicit return type annotation enhances type safety and resolves mypy
warn_return_anyerrors as noted in the PR objectives.
47-48: LGTM: Return statement aligns with type contract.Returning
resultwhen a task is skipped is correct for Ansible action plugins and satisfies thedict[str, Any]return type annotation. This ensures the plugin always returns a dictionary as expected by Ansible's action plugin interface.
- Move badge below title in README.md (MD041) - Add trailing newlines to CODE_OF_CONDUCT.md and PR template (MD047) - Add heading to pull_request_template.md (MD041) - Clean up .markdownlint.yaml config and remove unnecessary exceptions
Replace Poetry with uv across all workflows, Dockerfile, and project
configuration. This provides faster dependency resolution and a more
modern Python packaging workflow.
Summary by CodeRabbit
Chores
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.