support infrahub.yml or infrahub.yaml #490
Conversation
WalkthroughThe changes introduce dynamic discovery of the repository configuration file by implementing functions to locate Changes
Sequence Diagram(s)sequenceDiagram
participant CLI/Plugin/Generator
participant find_repository_config_file
participant get/load_repository_config
CLI/Plugin/Generator->>find_repository_config_file: Locate config file (.infrahub.yml/.yaml)
find_repository_config_file-->>CLI/Plugin/Generator: Return config file path
CLI/Plugin/Generator->>get/load_repository_config: Load config from located path
get/load_repository_config-->>CLI/Plugin/Generator: Parsed repository config
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying infrahub-sdk-python with
|
| Latest commit: |
1f2fc2b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ed0480f8.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pmc-yaml-yml.infrahub-sdk-python.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
🔭 Outside diff range comments (2)
infrahub_sdk/ctl/cli_commands.py (1)
23-23: Remove unused import.The pipeline indicates that the
configimport is no longer used since static path references were replaced with dynamic discovery.-from ..ctl import configinfrahub_sdk/pytest_plugin/utils.py (1)
1-1: Add missing future annotations import.The pipeline failure indicates that PEP 604 union syntax (
Path | None) requires the future annotations import.+from __future__ import annotations + from pathlib import Path
🧹 Nitpick comments (1)
infrahub_sdk/ctl/repository.py (1)
27-52: Well-implemented function with potential code duplication concern.The function logic is correct and well-documented:
- Properly searches for both
.infrahub.ymland.infrahub.yaml- Sensibly prefers
.ymlextension when both exist- Includes comprehensive docstring with type hints
However, this appears to be identical to
find_repository_config_fileininfrahub_sdk/pytest_plugin/utils.py. Consider consolidating these duplicate implementations into a shared utility module to maintain DRY principles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
infrahub_sdk/ctl/check.py(2 hunks)infrahub_sdk/ctl/cli_commands.py(5 hunks)infrahub_sdk/ctl/config.py(2 hunks)infrahub_sdk/ctl/generator.py(2 hunks)infrahub_sdk/ctl/repository.py(1 hunks)infrahub_sdk/pytest_plugin/plugin.py(3 hunks)infrahub_sdk/pytest_plugin/utils.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
infrahub_sdk/pytest_plugin/plugin.py (1)
infrahub_sdk/pytest_plugin/utils.py (2)
find_repository_config_file(9-33)load_repository_config(36-57)
infrahub_sdk/ctl/repository.py (2)
infrahub_sdk/pytest_plugin/utils.py (1)
find_repository_config_file(9-33)infrahub_sdk/schema/repository.py (1)
InfrahubRepositoryConfig(174-259)
🪛 GitHub Actions: CI
infrahub_sdk/ctl/cli_commands.py
[error] 23-23: Ruff: ..ctl.config imported but unused (F401). Remove unused import.
infrahub_sdk/ctl/generator.py
[error] 9-9: Ruff: ..ctl.config imported but unused (F401). Remove unused import.
infrahub_sdk/ctl/check.py
[error] 14-14: Ruff: ..ctl.config imported but unused (F401). Remove unused import.
infrahub_sdk/pytest_plugin/utils.py
[error] 9-9: Ruff: Missing from __future__ import annotations, but uses PEP 604 union (FA102). Add from __future__ import annotations.
🔇 Additional comments (17)
infrahub_sdk/ctl/config.py (2)
15-15: LGTM!Good addition of the alternate config file constant to support both
.ymland.yamlextensions.
73-73: LGTM!Nice catch fixing the typo in the docstring from "ValidationErorr" to "ValidationError".
infrahub_sdk/ctl/generator.py (2)
11-11: LGTM!Good addition of the
find_repository_config_fileimport to support dynamic config file discovery.
29-29: LGTM!Excellent improvement replacing the static config file path with dynamic discovery. This enables support for both
.infrahub.ymland.infrahub.yamlextensions.infrahub_sdk/ctl/check.py (2)
17-17: LGTM!Good addition of
find_repository_config_fileimport for dynamic config discovery.
62-62: LGTM!Excellent change replacing the static config path with dynamic discovery to support both config file extensions.
infrahub_sdk/ctl/cli_commands.py (5)
33-33: LGTM!Good addition of
find_repository_config_fileimport to enable dynamic config discovery.
263-263: LGTM!Excellent improvement replacing the static config path with dynamic discovery in the render command.
313-313: LGTM!Excellent improvement replacing the static config path with dynamic discovery in the transform command.
273-273: LGTM!Good improvement making the error message generic by removing the hardcoded config filename reference.
472-472: LGTM!Nice catch fixing the typo in the comment from "planel" to "panel".
infrahub_sdk/pytest_plugin/utils.py (2)
9-34: LGTM!Well-implemented function for dynamic config file discovery. The logic correctly prefers
.ymlover.yamlwhen both exist, and the error handling approach of returning the.ymlpath for consistent error messages is thoughtful.
37-47: LGTM!Excellent fallback logic enhancement. The bidirectional fallback between
.ymland.yamlextensions provides robust config file discovery while maintaining backward compatibility.infrahub_sdk/pytest_plugin/plugin.py (3)
12-12: LGTM!The import addition of
find_repository_config_fileis necessary for the new dynamic config file discovery functionality.
21-23: LGTM!The changes correctly implement dynamic config file discovery:
- Setting
default=Noneenables the new logic to automatically find the config file- The updated help text accurately describes support for both
.infrahub.ymland.infrahub.yamlextensions
66-69: LGTM!The conditional logic correctly implements dynamic config file discovery:
- Preserves existing behavior when
--infrahub-repo-configis explicitly provided- Uses
find_repository_config_file()for automatic discovery when no option is given- Maintains backward compatibility while adding support for both
.ymland.yamlextensionsinfrahub_sdk/ctl/repository.py (1)
55-65: LGTM! Enhanced fallback logic and error messaging.The changes provide useful enhancements:
- Fallback logic (lines 55-65): Automatically tries alternate extensions when the specified config file doesn't exist, improving compatibility
- Improved error message (line 69): Clearly indicates that both
.infrahub.ymland.infrahub.yamlextensions were checkedThis maintains backward compatibility while making the system more resilient to different naming conventions.
Also applies to: 69-69
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## infrahub-develop #490 +/- ##
====================================================
- Coverage 76.24% 75.52% -0.72%
====================================================
Files 100 100
Lines 9032 8881 -151
Branches 1731 1748 +17
====================================================
- Hits 6886 6707 -179
- Misses 1670 1691 +21
- Partials 476 483 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Related: opsmill/infrahub#6990 |
|
Do we need to update these as well?
|
Already done here: opsmill/infrahub#6990 |
|
This PR should target the |
Had to do a separate PR since this was already merged: #492 |
3 participants
Fix for opsmill/infrahub#6989
Summary by CodeRabbit
.infrahub.ymland.infrahub.yamlextensions.