-
Notifications
You must be signed in to change notification settings - Fork 6
Support infrahub.yml or infrahub.yaml configuration files #492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add find_repository_config_file() function to search for both .yml and .yaml extensions - Update all modules to use dynamic config file discovery - Prefer .yml over .yaml when both exist for backward compatibility - Update pytest plugin to support both extensions - Improve error messages to mention both possible file extensions
WalkthroughThe changes introduce dynamic discovery of the repository configuration file, supporting both Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI/Plugin/Generator
participant RepositoryUtils
User->>CLI/Plugin/Generator: Run command / start session
CLI/Plugin/Generator->>RepositoryUtils: find_repository_config_file()
RepositoryUtils->>RepositoryUtils: Check for .infrahub.yml and .infrahub.yaml
RepositoryUtils-->>CLI/Plugin/Generator: Return config file path (if found)
CLI/Plugin/Generator->>RepositoryUtils: get_repository_config(config_path)
RepositoryUtils->>RepositoryUtils: If not found, try alternate extension
RepositoryUtils-->>CLI/Plugin/Generator: Return config or error
CLI/Plugin/Generator-->>User: Proceed or report error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous 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). (3)
✨ 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: |
06905fa
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://207c9b4d.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pmc-yaml-2.infrahub-sdk-python.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🔭 Outside diff range comments (2)
infrahub_sdk/ctl/check.py (1)
223-223: Bug: malformed JSON string printedThis prints invalid JSON (duplicated quotes). Replace with a valid JSON payload.
- print('{"level": "WARNING", "message": "message", ""No check found"}') + print('{"level": "WARNING", "message": "No check found"}')infrahub_sdk/pytest_plugin/plugin.py (1)
66-73: Leverage loader fallback when path is providedCurrent logic only loads when the file exists, bypassing the loader’s alternate-extension fallback if the user passes a non-existent path. Delegate existence checks to
load_repository_configand ignoreFileNotFoundErrorto preserve the previous “optional config” behavior.- if session.config.option.infrahub_repo_config: - session.infrahub_config_path = Path(session.config.option.infrahub_repo_config) # type: ignore[attr-defined] - else: - session.infrahub_config_path = find_repository_config_file() # type: ignore[attr-defined] - - if session.infrahub_config_path.is_file(): # type: ignore[attr-defined] - session.infrahub_repo_config = load_repository_config(repo_config_file=session.infrahub_config_path) # type: ignore[attr-defined] + if session.config.option.infrahub_repo_config: + candidate = Path(session.config.option.infrahub_repo_config) + else: + candidate = find_repository_config_file() + try: + session.infrahub_repo_config = load_repository_config(repo_config_file=candidate) # type: ignore[attr-defined] + session.infrahub_config_path = candidate # type: ignore[attr-defined] + except FileNotFoundError: + # No repository config present; continue without loading + pass
🧹 Nitpick comments (6)
infrahub_sdk/ctl/cli_commands.py (1)
273-275: Nit: include more context in the error messageConsider including the repository config filename or directory to aid debugging when a transform isn’t found.
infrahub_sdk/ctl/repository.py (3)
27-51: Docstring claims a FileNotFoundError but function never raisesThe implementation returns a non-existent
.infrahub.ymlpath when neither file exists (for downstream error handling), so the "Raises" section is misleading. Either update the docstring or change the function to actually raise.Suggested docstring tweak:
def find_repository_config_file(base_path: Path | None = None) -> Path: - """Find the repository config file, checking for both .yml and .yaml extensions. + """Find the repository config file (.infrahub.yml or .infrahub.yaml). - Args: - base_path: Base directory to search in. If None, uses current directory. + - Prefers .yml if both exist. + - If neither exists, returns the .infrahub.yml path (even if missing) so callers can emit a consistent error. + + Args: + base_path: Base directory to search in. If None, uses the current directory. Returns: - Path to the config file. - - Raises: - FileNotFoundError: If neither .infrahub.yml nor .infrahub.yaml exists. + Path to the discovered config file, or the default .infrahub.yml path if none exists. """
27-51: Avoid code duplication: centralize config-file discoveryThis function duplicates nearly the same logic in infrahub_sdk/pytest_plugin/utils.py. Consider moving a single implementation to a shared, dependency-safe module (e.g., infrahub_sdk/utils/repository_config.py) and import it from both places to prevent drift.
55-65: Simplify and generalize extension fallbackThe current fallback checks exact filenames only. Using Path.with_suffix makes this shorter and more general (works even if the file is not named exactly ".infrahub.*").
- # If the file doesn't exist, try to find it with alternate extension - if not repo_config_file.exists(): - if repo_config_file.name == ".infrahub.yml": - alt_path = repo_config_file.parent / ".infrahub.yaml" - if alt_path.exists(): - repo_config_file = alt_path - elif repo_config_file.name == ".infrahub.yaml": - alt_path = repo_config_file.parent / ".infrahub.yml" - if alt_path.exists(): - repo_config_file = alt_path + # If the file doesn't exist, try same stem with the alternate extension + if not repo_config_file.exists(): + suffix = repo_config_file.suffix + if suffix in (".yml", ".yaml"): + alt_suffix = ".yaml" if suffix == ".yml" else ".yml" + alt_path = repo_config_file.with_suffix(alt_suffix) + if alt_path.exists(): + repo_config_file = alt_pathinfrahub_sdk/pytest_plugin/utils.py (2)
9-33: Docstring states it raises, but function does notLike the CLI counterpart, this helper returns a non-existent
.infrahub.ymlpath when neither file exists; it does not raise. Update the docstring to avoid misleading callers.def find_repository_config_file(base_path: Path | None = None) -> Path: - """Find the repository config file, checking for both .yml and .yaml extensions. + """Find the repository config file (.infrahub.yml or .infrahub.yaml). - Args: - base_path: Base directory to search in. If None, uses current directory. + - Prefers .yml if both exist. + - If neither exists, returns the .infrahub.yml path (even if missing) so callers can emit a consistent error. + + Args: + base_path: Base directory to search in. If None, uses the current directory. Returns: - Path to the config file. - - Raises: - FileNotFoundError: If neither .infrahub.yml nor .infrahub.yaml exists. + Path to the discovered config file, or the default .infrahub.yml path if none exists. """
36-47: Reduce duplication in extension fallbackUse Path.with_suffix to simplify and generalize the alternate-extension lookup. This mirrors the suggestion in the CLI module.
- # If the file doesn't exist, try to find it with alternate extension - if not repo_config_file.exists(): - if repo_config_file.name == ".infrahub.yml": - alt_path = repo_config_file.parent / ".infrahub.yaml" - if alt_path.exists(): - repo_config_file = alt_path - elif repo_config_file.name == ".infrahub.yaml": - alt_path = repo_config_file.parent / ".infrahub.yml" - if alt_path.exists(): - repo_config_file = alt_path + # If the file doesn't exist, try same stem with the alternate extension + if not repo_config_file.exists(): + suffix = repo_config_file.suffix + if suffix in (".yml", ".yaml"): + alt_suffix = ".yaml" if suffix == ".yml" else ".yml" + alt_path = repo_config_file.with_suffix(alt_suffix) + if alt_path.exists(): + repo_config_file = alt_path
📜 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(4 hunks)infrahub_sdk/ctl/config.py(1 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 (3)
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)
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/pytest_plugin/utils.py (2)
infrahub_sdk/ctl/repository.py (1)
find_repository_config_file(27-51)infrahub_sdk/schema/repository.py (1)
InfrahubRepositoryConfig(174-259)
🪛 GitHub Actions: CI
infrahub_sdk/ctl/check.py
[error] 14-14: 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/cli_commands.py
[error] 23-23: 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 (6)
infrahub_sdk/ctl/config.py (1)
15-15: LGTM: Added alternate repo config filename constantThe
.yamlalternate is correctly introduced alongside the existing.yml.infrahub_sdk/ctl/cli_commands.py (2)
263-264: LGTM: dynamic repo config discoverySwitching to
find_repository_config_file()improves robustness and aligns with the new behavior.
313-314: LGTM: dynamic repo config discovery (transform command)Consistent with the render path; looks good.
infrahub_sdk/pytest_plugin/plugin.py (2)
12-12: LGTM: import discovery utilitiesImporting both discovery and loading helpers is aligned with the new approach.
21-24: LGTM: CLI option default and help textDefaulting to None and documenting both extensions improves UX.
infrahub_sdk/ctl/repository.py (1)
69-69: Clearer error message — LGTMGood improvement referencing both .infrahub.yml and .infrahub.yaml for user clarity.
| logging.basicConfig(level=log_level, format=FORMAT, datefmt="[%X]", handlers=[RichHandler()]) | ||
|
|
||
| repository_config = get_repository_config(Path(config.INFRAHUB_REPO_CONFIG_FILE)) | ||
| repository_config = get_repository_config(find_repository_config_file()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Discover config relative to the provided path
To support running checks from arbitrary roots, consider passing the CLI path as base (if supported).
Confirm the repository helper supports a base path:
Proposed change:
- repository_config = get_repository_config(find_repository_config_file())
+ repository_config = get_repository_config(find_repository_config_file(base_path=Path(path)))🏁 Script executed:
#!/bin/bash
# Inspect the helper signature
rg -n "def find_repository_config_file\(" -A 2 infrahub_sdk/ctl/repository.py || trueLength of output: 246
Use the CLI’s path as the base when locating the repository config
The helper find_repository_config_file already accepts a base_path, so you can pass the CLI-provided path to ensure checks run correctly from any working directory.
• File: infrahub_sdk/ctl/check.py
• Line: 62
Proposed change:
- repository_config = get_repository_config(find_repository_config_file())
+ repository_config = get_repository_config(
+ find_repository_config_file(base_path=Path(path))
+ )📝 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.
| repository_config = get_repository_config(find_repository_config_file()) | |
| repository_config = get_repository_config( | |
| find_repository_config_file(base_path=Path(path)) | |
| ) |
🤖 Prompt for AI Agents
In infrahub_sdk/ctl/check.py at line 62, the call to find_repository_config_file
should pass the CLI-provided path as the base_path argument to ensure the
repository config is located relative to the CLI's current path. Modify the call
to find_repository_config_file to include base_path=path, where path is the CLI
input, so the repository_config is correctly resolved regardless of the working
directory.
| ) -> None: | ||
| init_logging(debug=debug) | ||
| repository_config = get_repository_config(Path(config.INFRAHUB_REPO_CONFIG_FILE)) | ||
| repository_config = get_repository_config(find_repository_config_file()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Optional: allow discovery from a custom base path
You accept a path argument; consider using it as the search base if find_repository_config_file supports it. This improves portability when running from subdirs.
Please verify the function signature supports a base path:
If supported, the change would look like:
- repository_config = get_repository_config(find_repository_config_file())
+ repository_config = get_repository_config(find_repository_config_file(base_path=Path(path)))🏁 Script executed:
#!/bin/bash
# Check signature of find_repository_config_file in ctl/repository.py
rg -n "def find_repository_config_file\(" -A 2 infrahub_sdk/ctl/repository.py || trueLength of output: 246
Support custom base path for repository config discovery
The find_repository_config_file function already accepts a base_path: Path | None parameter. To honor the caller’s path argument when locating the config file, update the call accordingly:
- Import
Pathif it isn’t already:from pathlib import Path
- Change the instantiation to pass the provided
pathasbase_path:
- repository_config = get_repository_config(find_repository_config_file())
+ repository_config = get_repository_config(
+ find_repository_config_file(base_path=Path(path))
+ )This ensures that running the CLI from a subdirectory uses the intended search root.
🤖 Prompt for AI Agents
In infrahub_sdk/ctl/generator.py at line 29, the call to
find_repository_config_file should pass the caller's path argument as the
base_path parameter to support custom base path discovery. Import Path from
pathlib if not already imported, then update the call to
find_repository_config_file by passing base_path=path (assuming path is the
variable holding the caller's path). This change ensures the repository config
is located relative to the intended directory.
|
Relates to #490 |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## infrahub-develop #492 +/- ##
=====================================================
- Coverage 70.92% 48.18% -22.74%
=====================================================
Files 87 100 +13
Lines 7879 8903 +1024
Branches 1527 1661 +134
=====================================================
- Hits 5588 4290 -1298
- Misses 1897 4234 +2337
+ Partials 394 379 -15
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 58 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
.infrahub.ymland.infrahub.yamlrepository configuration files, with automatic detection and preference for.infrahub.ymlif both exist.Bug Fixes
Chores
.ymland.yamlconfig file extensions.