Skip to content

Make --list-sources respect --output-type and --output-file - #376

Open
rjeffman wants to merge 2 commits into
freeipa:masterfrom
rjeffman:list_sources_json
Open

Make --list-sources respect --output-type and --output-file#376
rjeffman wants to merge 2 commits into
freeipa:masterfrom
rjeffman:list_sources_json

Conversation

@rjeffman

@rjeffman rjeffman commented Apr 1, 2026

Copy link
Copy Markdown
Member

Refactor list_sources() to return structured data instead of printing directly to stdout. Route output through the output plugin system so that --output-type controls the format (json, human, prometheus) and --output-file controls the destination.

This allows users to get machine-readable JSON output of available sources and checks, making it easier to programmatically query what checks are available.

The human output format remains unchanged from the original plain text format. JSON produces an array of {source, checks} objects. Prometheus raises an error as source listing has no meaningful metric representation.

Assisted-by: Claude Sonnet 4.5 noreply@anthropic.com

Summary by Sourcery

Route source listing through the output plugin system so it respects configured output type and destination.

New Features:

  • Support structured source listing output via JSON and human output plugins, controlled by --output-type and --output-file.

Enhancements:

  • Refactor list_sources() to return structured source/check data instead of printing directly to stdout.
  • Disallow using --list-sources with Prometheus output and surface a clear error message from both the CLI and Prometheus output plugin.

Tests:

  • Add a CLI options test asserting that --list-sources with --output-type=prometheus raises a ValueError.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Calling output.render_source_list with the Prometheus output will now raise a RuntimeError that appears unhandled at the CLI level; consider either blocking --output-type=prometheus when --list-sources is used or catching this exception and turning it into a clean user-facing error message/exit code.
  • The new list_sources now returns a list of dicts built from a dict and then iterated, which preserves insertion order but is an implicit dependency on dict ordering; if stable or sorted output is important (especially for JSON consumers) consider explicitly sorting by source and/or check name before returning.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Calling `output.render_source_list` with the Prometheus output will now raise a `RuntimeError` that appears unhandled at the CLI level; consider either blocking `--output-type=prometheus` when `--list-sources` is used or catching this exception and turning it into a clean user-facing error message/exit code.
- The new `list_sources` now returns a list of dicts built from a `dict` and then iterated, which preserves insertion order but is an implicit dependency on dict ordering; if stable or sorted output is important (especially for JSON consumers) consider explicitly sorting by source and/or check name before returning.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@rjeffman
rjeffman marked this pull request as draft April 1, 2026 14:14
@rjeffman
rjeffman force-pushed the list_sources_json branch from 50d3c9c to ab19e31 Compare April 1, 2026 14:31
@rjeffman
rjeffman marked this pull request as ready for review April 1, 2026 14:32

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In Human.generate_source_list, the check lines now have three leading spaces instead of two (' %s' vs previously " ", name); if consumers rely on the exact human-readable layout, consider adjusting the indentation to truly preserve the legacy format as claimed.
  • The Prometheus backend both raises at generate_source_list and is preemptively rejected in parse_options; consider picking a single enforcement layer (either only the option validation or only the backend RuntimeError) to avoid redundant checks and keep the responsibility boundary clearer.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `Human.generate_source_list`, the check lines now have three leading spaces instead of two (`'   %s'` vs previously `"  ", name`); if consumers rely on the exact human-readable layout, consider adjusting the indentation to truly preserve the legacy format as claimed.
- The Prometheus backend both raises at `generate_source_list` and is preemptively rejected in `parse_options`; consider picking a single enforcement layer (either only the option validation or only the backend RuntimeError) to avoid redundant checks and keep the responsibility boundary clearer.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Refactor list_sources() to return structured data instead of printing
directly to stdout. Route output through the output plugin system so
that --output-type controls the format (json, human, prometheus) and
--output-file controls the destination.

This allows users to get machine-readable JSON output of available
sources and checks, making it easier to programmatically query what
checks are available.

The human output format remains unchanged from the original plain text
format. JSON produces an array of {source, checks} objects. Prometheus
raises an error as source listing has no meaningful metric representation.

Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
@rjeffman
rjeffman force-pushed the list_sources_json branch 2 times, most recently from d8ed549 to 268c432 Compare April 1, 2026 14:54
Verifies that the incompatible combination of --list-sources with
--output-type=prometheus raises a clear error message.

Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
@rjeffman
rjeffman force-pushed the list_sources_json branch from 268c432 to f58f464 Compare April 1, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant