Migrate print statements to structured logging#117
Open
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request migrates the codebase from using print statements to the Python logging module. Changes include adding module-level loggers across adapter modules (infrahub, nautobot, netbox, prometheus, ACI, genericrestapi, ipfabricsync, slurpitsync, and rest_api_client) and the core CLI, replacing all user-facing and diagnostic print calls with logger.info, logger.warning, logger.debug, and logger.exception calls. The CLI receives enhanced logging configuration with a new Verbosity enum and setup function to manage log levels. The Potenda class and utility functions receive updated signatures to accept verbosity parameters. Documentation examples and tasks are updated to use uv instead of poetry. A new test module validates logging conventions across the codebase. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
infrahub_sync/__init__.py (1)
3-15:⚠️ Potential issue | 🟠 MajorMove logger initialization after imports to resolve E402 linting error
The
loggerinitialization on line 8 occurs between standard library imports (lines 3-6) and third-party imports (lines 10-15), violating PEP 8 and Ruff's E402 rule. Relocate the logger setup after all imports complete.Proposed fix
import logging import operator import re from typing import Any, Union - -logger = logging.getLogger(__name__) import pydantic from diffsync.enum import DiffSyncFlags @@ -15,3 +13,5 @@ from netutils.ip import is_ip_within as netutils_is_ip_within from packaging import version from infrahub_sync.adapters.utils import get_value + +logger = logging.getLogger(__name__)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sync/__init__.py` around lines 3 - 15, Move the module-level logger initialization so it occurs after all imports to satisfy E402; specifically, remove or relocate the line that assigns logger = logging.getLogger(__name__) out of the import block and place it immediately after the last import (after symbols like pydantic, DiffSyncFlags, StrictUndefined, NativeEnvironment, netutils_is_ip_within, and version are imported) so the logger variable is initialized post-imports.infrahub_sync/adapters/ipfabricsync.py (1)
11-29:⚠️ Potential issue | 🟠 MajorFix import order and fail fast on missing
ipfabricdependencyThis block has three Ruff violations that must be resolved:
- TRY400: Use
logging.exceptioninstead oflogging.errorin the except block (line 18)- E402: Module-level imports at lines 20, 22, 29 are placed after logger assignment
More critically, swallowing the
ImportErrorallows execution to continue, causing a runtimeNameErrorlater whenIPFClientis used (lines 51, 68). Re-raise the exception to fail fast.Proposed fix
from __future__ import annotations import os from typing import TYPE_CHECKING, Any try: from typing import Self except ImportError: from typing_extensions import Self import json import logging try: from ipfabric import IPFClient -except ImportError as e: - logger.error("Failed to import ipfabric: %s", e) +except ImportError as exc: + logging.getLogger(__name__).exception("Failed to import ipfabric dependency") + msg = "Missing optional dependency 'ipfabric'. Install it to use IpfabricsyncAdapter." + raise ImportError(msg) from exc from diffsync import Adapter, DiffSyncModel from infrahub_sync import ( DiffSyncMixin, DiffSyncModelMixin, SchemaMappingModel, SyncAdapter, SyncConfig, ) from infrahub_sync.adapters.utils import build_mapping if TYPE_CHECKING: from collections.abc import Mapping + +logger = logging.getLogger(__name__)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sync/adapters/ipfabricsync.py` around lines 11 - 29, Reorder the module imports so all top-level imports (diffsync.Adapter, DiffSyncModel, DiffSyncMixin, DiffSyncModelMixin, SchemaMappingModel, SyncAdapter, SyncConfig, build_mapping) occur before creating the module logger, and change the ipfabric import block to log and fail fast: when importing IPFClient, catch ImportError, call logger.exception(...) to record the traceback, then re-raise the exception so execution stops instead of allowing later NameError when IPFClient is referenced; update the try/except around "from ipfabric import IPFClient" and ensure logger = logging.getLogger(__name__) remains after the standard imports are in correct order.infrahub_sync/utils.py (1)
9-19:⚠️ Potential issue | 🟠 MajorMove logger initialization below all imports (E402).
Declaring
loggerbefore third-party/local imports causes Ruff E402 failures.💡 Suggested fix
import importlib.util import logging import sys from pathlib import Path from typing import TYPE_CHECKING, Union -logger = logging.getLogger(__name__) - import yaml from diffsync.store.local import LocalStore from diffsync.store.redis import RedisStore from infrahub_sdk import Config @@ from infrahub_sync.plugin_loader import PluginLoader, PluginLoadError from infrahub_sync.potenda import Potenda + +logger = logging.getLogger(__name__)As per coding guidelines: "Format Python code with Ruff and ensure lint-clean output".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sync/utils.py` around lines 9 - 19, The logger variable is declared before module imports causing a Ruff E402 import-order error; move the line initializing logger (logger = logging.getLogger(__name__)) so it appears after all import statements (after imports like yaml, LocalStore, RedisStore, Config, SyncAdapter, SyncConfig, SyncInstance, render_template, PluginLoader, PluginLoadError, Potenda) to satisfy import ordering rules.infrahub_sync/adapters/slurpitsync.py (1)
5-12:⚠️ Potential issue | 🟠 MajorNormalize imports; current block fails Ruff and duplicates
Any.The current import section is unsorted and re-imports
Anyin the fallback block.💡 Suggested fix
import asyncio import ipaddress +import logging from typing import TYPE_CHECKING, Any try: - from typing import Any, Self + from typing import Self except ImportError: - from typing_extensions import Any, Self + from typing_extensions import Self + import slurpit @@ -import logging - logger = logging.getLogger(__name__)As per coding guidelines: "Format Python code with Ruff and ensure lint-clean output".
Also applies to: 23-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sync/adapters/slurpitsync.py` around lines 5 - 12, The import block currently duplicates Any and is unsorted; replace it with a single, ruff-compliant import section that imports TYPE_CHECKING and Any from typing and conditionally imports Self (use typing.Self if available, otherwise import Self from typing_extensions) using a try/except only for Self; remove the duplicate Any and then sort/group imports (stdlib, third-party) so symbols like TYPE_CHECKING, Any, Self, slurpit, Adapter, and DiffSyncModel are imported once each and in proper order; apply the same normalization to the second duplicate import block around symbols on lines 23-25.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrahub_sync/adapters/genericrestapi.py`:
- Around line 27-30: Move the import logging statement so it appears before the
existing if TYPE_CHECKING: block to match the other adapters; ensure the
module-level logger variable (logger = logging.getLogger(__name__)) remains
directly after the logging import and before any TYPE_CHECKING or typing-only
imports so the logger is initialized consistently with
aci.py/nautobot.py/netbox.py/prometheus.py.
In `@infrahub_sync/adapters/infrahub.py`:
- Around line 33-35: Move the standard-library import "import logging" into the
top group of imports (above or with other stdlib imports) so it isn't after
local package imports; update the import order in
infrahub_sync/adapters/infrahub.py so the stdlib import precedes local imports
and keep "logger = logging.getLogger(__name__)" unchanged, ensuring the file is
Ruff/I001-compliant (or run isort/ruff to auto-fix).
In `@infrahub_sync/adapters/nautobot.py`:
- Around line 23-27: The import order is wrong: the standard library import
logging must come before the local import of get_value to satisfy Ruff I001;
move the line "import logging" above "from .utils import get_value", keep the
logger = logging.getLogger(__name__) line intact, and ensure imports are grouped
as stdlib, third-party, then local so Ruff lint passes (re-run ruff/formatting
after change).
In `@infrahub_sync/adapters/netbox.py`:
- Around line 24-29: The import ordering in netbox.py is incorrect: move the
stdlib import 'import logging' so it sits with other stdlib imports before the
relative import from '.utils' to satisfy Ruff's import-order rule; update the
top of the file so 'import logging' appears above 'from .utils import get_value'
(the logger = logging.getLogger(__name__) line can remain unchanged).
In `@infrahub_sync/adapters/prometheus.py`:
- Around line 25-28: Move the import logging statement so it appears immediately
after the typing_extensions import and before any third-party imports in this
module (same place pattern used in aci.py); update the top-of-file import order
in infrahub_sync.adapters.prometheus so that logging is imported before creating
logger = logging.getLogger(__name__) and follows the standard stdlib ->
third-party -> local import grouping enforced by Ruff.
In `@infrahub_sync/adapters/rest_api_client.py`:
- Around line 92-95: The except block catching
requests.exceptions.JSONDecodeError currently logs the raw response body; change
it to avoid logging response.text and instead call logger.exception with a
contextual message that includes response.url, response.status_code, and
response.headers.get('Content-Type') (or response.headers) to capture traceback
without leaking PII; keep raising the ValueError from exc (raise ValueError(msg)
from exc) and ensure the logged message does not contain the response body.
- Around line 5-10: Add "from __future__ import annotations" at the top and fix
the import grouping in rest_api_client.py so standard-library imports are
contiguous: place "from typing import Any" directly below "import logging" with
no blank line, then a blank line before third-party imports (requests); this
will make the module-level imports (including the logger variable) comply with
Ruff I001.
In `@infrahub_sync/adapters/slurpitsync.py`:
- Line 213: The log call at logger.info("%s: Loading all %d %s", self.type,
total, resource_name) can raise NameError because resource_name isn't defined
for non-dotted mappings (e.g., planning_results, filter_interfaces, adapter
methods); update the code that emits this log (look for the logger.info call and
surrounding logic in slurpitsync.SlurpItSync) to compute a safe fallback for
resource_name (for example obtain resource_name via getattr(self,
"resource_name", None) or derive it from the mapping metadata) and use that
fallback (or self.type) when resource_name is missing so the log never
references an undefined variable.
In `@infrahub_sync/cli.py`:
- Around line 57-59: The root command currently checks ctx.invoked_subcommand
but calls ctx.get_help() (which only returns help text); change this to print
the help and exit: call click.echo(ctx.get_help()) (or print the returned
string) and then call ctx.exit(0) so the CLI shows the help when no subcommand
is invoked; update the block that checks ctx.invoked_subcommand in
infrahub_sync/cli.py accordingly.
In `@tests/test_logging.py`:
- Around line 67-79: The exemption currently checks only py_file.name via
_LOGGER_EXEMPT, which skips every __init__.py; update the exemption logic so it
matches full relative paths instead of just filename: change _LOGGER_EXEMPT to
contain the specific relative module paths you want to allow (e.g.,
"some/package/__init__.py") and in test_modules_have_logger() compute the file's
repo-relative path (via _python_files() result relative to the repository root)
and compare that string/Path against _LOGGER_EXEMPT before continuing; this
keeps the filename check but prevents every __init__.py from being globally
exempted (reference _LOGGER_EXEMPT, test_modules_have_logger(), and
_python_files()).
---
Outside diff comments:
In `@infrahub_sync/__init__.py`:
- Around line 3-15: Move the module-level logger initialization so it occurs
after all imports to satisfy E402; specifically, remove or relocate the line
that assigns logger = logging.getLogger(__name__) out of the import block and
place it immediately after the last import (after symbols like pydantic,
DiffSyncFlags, StrictUndefined, NativeEnvironment, netutils_is_ip_within, and
version are imported) so the logger variable is initialized post-imports.
In `@infrahub_sync/adapters/ipfabricsync.py`:
- Around line 11-29: Reorder the module imports so all top-level imports
(diffsync.Adapter, DiffSyncModel, DiffSyncMixin, DiffSyncModelMixin,
SchemaMappingModel, SyncAdapter, SyncConfig, build_mapping) occur before
creating the module logger, and change the ipfabric import block to log and fail
fast: when importing IPFClient, catch ImportError, call logger.exception(...) to
record the traceback, then re-raise the exception so execution stops instead of
allowing later NameError when IPFClient is referenced; update the try/except
around "from ipfabric import IPFClient" and ensure logger =
logging.getLogger(__name__) remains after the standard imports are in correct
order.
In `@infrahub_sync/adapters/slurpitsync.py`:
- Around line 5-12: The import block currently duplicates Any and is unsorted;
replace it with a single, ruff-compliant import section that imports
TYPE_CHECKING and Any from typing and conditionally imports Self (use
typing.Self if available, otherwise import Self from typing_extensions) using a
try/except only for Self; remove the duplicate Any and then sort/group imports
(stdlib, third-party) so symbols like TYPE_CHECKING, Any, Self, slurpit,
Adapter, and DiffSyncModel are imported once each and in proper order; apply the
same normalization to the second duplicate import block around symbols on lines
23-25.
In `@infrahub_sync/utils.py`:
- Around line 9-19: The logger variable is declared before module imports
causing a Ruff E402 import-order error; move the line initializing logger
(logger = logging.getLogger(__name__)) so it appears after all import statements
(after imports like yaml, LocalStore, RedisStore, Config, SyncAdapter,
SyncConfig, SyncInstance, render_template, PluginLoader, PluginLoadError,
Potenda) to satisfy import ordering rules.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
infrahub_sync/__init__.pyinfrahub_sync/adapters/aci.pyinfrahub_sync/adapters/genericrestapi.pyinfrahub_sync/adapters/infrahub.pyinfrahub_sync/adapters/ipfabricsync.pyinfrahub_sync/adapters/nautobot.pyinfrahub_sync/adapters/netbox.pyinfrahub_sync/adapters/prometheus.pyinfrahub_sync/adapters/rest_api_client.pyinfrahub_sync/adapters/slurpitsync.pyinfrahub_sync/cli.pyinfrahub_sync/potenda/__init__.pyinfrahub_sync/utils.pypyproject.tomltests/test_logging.py
Add .infrahub.yml with schemas directory reference so Infrahub git sync automatically installs schemas. Update CLAUDE.md with structured logging context from the implementation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Deploying infrahub-sync with
|
| Latest commit: |
c77cb56
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1ba6abe0.infrahub-sync.pages.dev |
| Branch Preview URL: | https://001-structured-logging.infrahub-sync.pages.dev |
BeArchiTek
changed the title
BeArchiTek
approved these changes
Mar 6, 2026
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tasks/linter.py (1)
61-67:⚠️ Potential issue | 🟡 MinorInconsistent working directory handling for
lint_yaml.The
lint_yamltask (lines 62-67) runscontext.run()directly without wrapping it inwith context.cd(ESCAPED_REPO_PATH):, unlikelint_ruff(lines 46-53) andlint_pylint(lines 36-42). SinceMAIN_DIRECTORY = ".", this causesyamllintto execute from the caller's current directory instead of the repository root.🔧 Suggested fix
`@task` def lint_yaml(context: Context) -> None: """This will run yamllint to validate formatting of all yaml files.""" print(f" - [{NAMESPACE}] Format yaml with yamllint") exec_cmd = f"yamllint {MAIN_DIRECTORY}" - context.run(exec_cmd, pty=True) + with context.cd(ESCAPED_REPO_PATH): + context.run(exec_cmd, pty=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tasks/linter.py` around lines 61 - 67, The lint_yaml task runs yamllint without changing into the repository root; wrap the context.run call in a with context.cd(ESCAPED_REPO_PATH): block (same pattern used in lint_ruff and lint_pylint) so yamllint is executed from the repo root; update the function lint_yaml to use ESCAPED_REPO_PATH and MAIN_DIRECTORY when calling context.run to mirror the other lint tasks.
🧹 Nitpick comments (3)
tasks/linter.py (1)
9-9: Consider consistency withtasks/tests.pypath definition.
tasks/tests.pystill usesMAIN_DIRECTORY = CURRENT_DIRECTORY.parent(an absolutePath), while this file now usesMAIN_DIRECTORY = "."(a relative string). This inconsistency could lead to confusion when maintaining the task definitions.If both files should resolve to the repository root, consider using a consistent approach across all task modules (either both use relative
"."or both derive fromCURRENT_DIRECTORY.parent).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tasks/linter.py` at line 9, The MAIN_DIRECTORY constant in tasks/linter.py is a relative string ("."), which is inconsistent with tasks/tests.py's MAIN_DIRECTORY = CURRENT_DIRECTORY.parent (a Path); update tasks/linter.py to use the same approach by setting MAIN_DIRECTORY to CURRENT_DIRECTORY.parent (or alternately change tasks/tests.py to ".") so both modules use the same type and resolve to the repository root consistently; locate the MAIN_DIRECTORY definition in tasks/linter.py and replace the string value with the CURRENT_DIRECTORY.parent Path used in tasks/tests.py.infrahub_sync/cli.py (2)
24-27: Consider adding a brief docstring to the public enum.Per coding guidelines, public classes require concise docstrings.
📝 Suggested enhancement
class Verbosity(str, Enum): + """Log verbosity levels for CLI output control.""" quiet = "quiet" default = "default" verbose = "verbose"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sync/cli.py` around lines 24 - 27, Add a concise docstring to the public enum class Verbosity explaining its purpose and the meaning of its members; update the class definition for Verbosity (subclassing Enum) to include a one- or two-sentence docstring that describes that it controls CLI logging/output level and briefly lists/mentions the members quiet, default, and verbose for clarity.
179-232: Logging migration looks good, consider adding context for consistency.The
logger.infousage is correct. However, unlikediff_cmdandsync_cmd, this command doesn't acceptctx: typer.Context. While not strictly necessary (since it doesn't propagate verbosity downstream), adding it would maintain signature consistency across commands.📝 Optional: Add context parameter for consistency
`@app.command`(name="generate") def generate( + ctx: typer.Context, name: str = typer.Option(default=None, help="Name of the sync to use"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sync/cli.py` around lines 179 - 232, The generate command should accept the Typer context like the other commands for signature consistency: add a first parameter ctx: typer.Context (or ctx: Context if imported) to the generate(...) function signature, keep it unused if not needed, and update any internal references if they rely on positional args; ensure the function still registers with `@app.command`(name="generate") and that imports include typer.Context if not already present so the signature matches diff_cmd and sync_cmd.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tasks/linter.py`:
- Around line 61-67: The lint_yaml task runs yamllint without changing into the
repository root; wrap the context.run call in a with
context.cd(ESCAPED_REPO_PATH): block (same pattern used in lint_ruff and
lint_pylint) so yamllint is executed from the repo root; update the function
lint_yaml to use ESCAPED_REPO_PATH and MAIN_DIRECTORY when calling context.run
to mirror the other lint tasks.
---
Nitpick comments:
In `@infrahub_sync/cli.py`:
- Around line 24-27: Add a concise docstring to the public enum class Verbosity
explaining its purpose and the meaning of its members; update the class
definition for Verbosity (subclassing Enum) to include a one- or two-sentence
docstring that describes that it controls CLI logging/output level and briefly
lists/mentions the members quiet, default, and verbose for clarity.
- Around line 179-232: The generate command should accept the Typer context like
the other commands for signature consistency: add a first parameter ctx:
typer.Context (or ctx: Context if imported) to the generate(...) function
signature, keep it unused if not needed, and update any internal references if
they rely on positional args; ensure the function still registers with
`@app.command`(name="generate") and that imports include typer.Context if not
already present so the signature matches diff_cmd and sync_cmd.
In `@tasks/linter.py`:
- Line 9: The MAIN_DIRECTORY constant in tasks/linter.py is a relative string
("."), which is inconsistent with tasks/tests.py's MAIN_DIRECTORY =
CURRENT_DIRECTORY.parent (a Path); update tasks/linter.py to use the same
approach by setting MAIN_DIRECTORY to CURRENT_DIRECTORY.parent (or alternately
change tasks/tests.py to ".") so both modules use the same type and resolve to
the repository root consistently; locate the MAIN_DIRECTORY definition in
tasks/linter.py and replace the string value with the CURRENT_DIRECTORY.parent
Path used in tasks/tests.py.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd3194ee-156b-4164-908b-f559c4cff4ea
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.infrahub.ymlCLAUDE.mdCLAUDE.mdinfrahub_sync/__init__.pyinfrahub_sync/adapters/genericrestapi.pyinfrahub_sync/adapters/infrahub.pyinfrahub_sync/adapters/ipfabricsync.pyinfrahub_sync/adapters/nautobot.pyinfrahub_sync/adapters/netbox.pyinfrahub_sync/adapters/prometheus.pyinfrahub_sync/adapters/rest_api_client.pyinfrahub_sync/adapters/slurpitsync.pyinfrahub_sync/cli.pyinfrahub_sync/utils.pypyproject.tomltasks/linter.pytests/test_logging.py
🚧 Files skipped from review as they are similar to previous changes (7)
- .infrahub.yml
- infrahub_sync/utils.py
- infrahub_sync/adapters/rest_api_client.py
- infrahub_sync/init.py
- pyproject.toml
- infrahub_sync/adapters/genericrestapi.py
- tests/test_logging.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
AGENTS.md (1)
222-222: Verify the example agent version reference.The example references "Claude Opus 4.6" which appears to be a hypothetical future version. Consider using a more generic placeholder or current version to avoid confusion.
📝 Suggested alternative
-- Agents must identify themselves (for example, `Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>` or `🤖 Generated with Copilot`). +- Agents must identify themselves (for example, `Co-Authored-By: Claude <noreply@anthropic.com>` or `🤖 Generated with Copilot`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` at line 222, Replace the hard-coded future version "Claude Opus 4.6" in the example agent-identification string with a generic or current-version placeholder to avoid implying a nonexistent release; update the example text `Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>` (found in AGENTS.md) to something like `Co-Authored-By: Claude Opus <noreply@anthropic.com>` or `Co-Authored-By: Claude Opus <version> <noreply@anthropic.com>` (or use an explicitly current version), and ensure the alternative `🤖 Generated with Copilot` example remains unchanged.tasks/docs.py (1)
22-22: Print statements remain despite PR objective to migrate to structured logging.The PR objective states "replace print statements with the logging system" for consistent, structured output. This file still uses
print()on lines 22, 34, 37, and 46. Consider whether these task runner messages should also be migrated to use the logging module for consistency with the rest of the codebase.If these are intentionally kept as
print()for invoke task output visibility, please clarify. Otherwise, the same logging pattern used elsewhere in the PR should apply here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tasks/docs.py` at line 22, Replace the bare print() calls in this module with structured logging: add "import logging" and create a module logger (logger = logging.getLogger(__name__)), then change each print("...") used for task runner messages to logger.info("...") so they follow the same logging pattern used across the PR; if the prints were intentionally left for invoke/task output, add a short comment explaining that instead of converting them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 98: The markdown list item "Available: `infrahub`, `netbox`, `nautobot`,
`aci`, `prometheus`, `peeringmanager`, `ipfabricsync`, `slurpitsync`,
`genericrestapi`" uses 2-space indentation causing MD007; update the indentation
to 4 spaces so the unordered list entry conforms to markdownlint rules (adjust
the leading spaces before the "-" or list marker to four spaces) and re-run the
linter to confirm the MD007 warning is resolved.
---
Nitpick comments:
In `@AGENTS.md`:
- Line 222: Replace the hard-coded future version "Claude Opus 4.6" in the
example agent-identification string with a generic or current-version
placeholder to avoid implying a nonexistent release; update the example text
`Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>` (found in AGENTS.md)
to something like `Co-Authored-By: Claude Opus <noreply@anthropic.com>` or
`Co-Authored-By: Claude Opus <version> <noreply@anthropic.com>` (or use an
explicitly current version), and ensure the alternative `🤖 Generated with
Copilot` example remains unchanged.
In `@tasks/docs.py`:
- Line 22: Replace the bare print() calls in this module with structured
logging: add "import logging" and create a module logger (logger =
logging.getLogger(__name__)), then change each print("...") used for task runner
messages to logger.info("...") so they follow the same logging pattern used
across the PR; if the prints were intentionally left for invoke/task output, add
a short comment explaining that instead of converting them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b923b44b-cb5e-460e-9bc5-c45b22390b13
📒 Files selected for processing (5)
AGENTS.mddocs/docs/adapters/aci.mdxdocs/docs/adapters/infrahub.mdxdocs/docs/adapters/prometheus.mdxtasks/docs.py
✅ Files skipped from review due to trivial changes (3)
- docs/docs/adapters/prometheus.mdx
- docs/docs/adapters/infrahub.mdx
- docs/docs/adapters/aci.mdx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #116
Summary by CodeRabbit
Release Notes
New Features
Documentation
uvpackage manager.Chores