-
Notifications
You must be signed in to change notification settings - Fork 258
Bugfix added sink_support to push scans #455
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
WalkthroughAn optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (main.py)
participant Config
participant Runner
participant HTTP Endpoint
User->>CLI (main.py): Run CLI with --named_sinks option
CLI (main.py)->>Config: Instantiate with named_sinks
CLI (main.py)->>Runner: Start scan, pass named_sinks
Runner->>Runner: Prepare result/error
Runner->>HTTP Endpoint: POST scan payload (includes named_sinks if provided)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
robusta_krr/core/models/config.py(1 hunks)robusta_krr/core/runner.py(5 hunks)robusta_krr/main.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
robusta_krr/core/runner.py (2)
robusta_krr/core/models/result.py (1)
Result(62-115)robusta_krr/formatters/json.py (1)
json(6-7)
🪛 Ruff (0.12.2)
robusta_krr/main.py
311-316: Do not perform function call typer.Option in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🪛 Flake8 (7.2.0)
robusta_krr/main.py
[error] 369-369: closing bracket does not match indentation of opening bracket's line
(E123)
🔇 Additional comments (9)
robusta_krr/core/models/config.py (1)
67-67: LGTM! Clean configuration field addition.The new
named_sinksfield is properly typed asOptional[list[str]]with a sensible default ofNone. The placement within the "Publishing to url settings" section is logical and consistent with the existing configuration structure.robusta_krr/main.py (2)
311-316: LGTM! Well-structured CLI option addition.The
named_sinksCLI option is properly implemented with:
- Correct type annotation
Optional[List[str]]- Clear help text explaining its purpose
- Appropriate placement in the "Publish Scan Settings" panel
- Consistent naming convention with other CLI options
368-368: LGTM! Parameter correctly passed to Config.The
named_sinksparameter is properly forwarded to theConfigconstructor, maintaining consistency with other configuration parameters.robusta_krr/core/runner.py (6)
9-9: LGTM! Import statement correctly updated.The
Listtype import is properly added to support the newnamed_sinksparameter typing.
523-525: LGTM! Method signature and call updated correctly.The
_send_resultmethod properly:
- Accepts the new
named_sinksparameter with correct typing- Forwards it to
_send_scan_payloadin the right position- Maintains consistency with the overall parameter flow
531-531: LGTM! Correctly uses settings for named_sinks.The
publish_errorfunction properly retrievesnamed_sinksfromsettings.named_sinks, maintaining consistency with how other settings are accessed.
547-554: LGTM! Function signature properly updated.The
_send_scan_payloadfunction signature correctly:
- Includes the new
named_sinksparameter with proper typing- Maintains the existing parameter order
- Uses appropriate Union types for flexible parameter handling
559-559: LGTM! Debug logging enhanced with sink information.The debug log message now includes the
named_sinksparameter, which will be helpful for troubleshooting scan payload routing.
576-577: LGTM! Conditional payload enhancement implemented correctly.The logic properly:
- Checks if
named_sinksis provided (truthy check handles bothNoneand empty list)- Adds the
"sinks"key to the action request only when sinks are specified- Uses the correct key name in the payload structure
arikalon1
left a comment
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.
nice work, left one comment
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: 0
♻️ Duplicate comments (1)
robusta_krr/main.py (1)
373-373: Fix the parameter order in the function call.The parameter order is still incorrect. Based on the function signature in
robusta_krr/core/runner.py(line 526-527), the correct order should be(url, scan_id, start_time, error, named_sinks).Apply this diff to fix the parameter order:
- publish_input_error( publish_scan_url, start_time, scan_id, str(e), named_sinks) + publish_input_error(publish_scan_url, scan_id, start_time, str(e), named_sinks)
🧹 Nitpick comments (1)
robusta_krr/main.py (1)
368-369: Fix indentation of closing bracket.The
named_sinksparameter addition is correct, but there's an indentation issue with the closing bracket.Apply this diff to fix the indentation:
named_sinks=named_sinks, - ) + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
robusta_krr/main.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
robusta_krr/main.py (2)
robusta_krr/core/models/config.py (2)
Config(20-192)set_config(177-188)robusta_krr/core/runner.py (1)
publish_input_error(527-528)
🪛 Ruff (0.12.2)
robusta_krr/main.py
311-316: Do not perform function call typer.Option in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🪛 Flake8 (7.2.0)
robusta_krr/main.py
[error] 369-369: closing bracket does not match indentation of opening bracket's line
(E123)
🔇 Additional comments (1)
robusta_krr/main.py (1)
311-316: LGTM! CLI parameter addition is correct.The
named_sinksparameter follows the established pattern for CLI options in this codebase. The static analysis hint B008 can be safely ignored as it's a false positive - usingtyper.Optionin argument defaults is the correct pattern for typer CLI applications.
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: 0
🧹 Nitpick comments (1)
robusta_krr/main.py (1)
368-369: Fix indentation of closing bracket.The
named_sinksparameter is correctly passed to the Config constructor, but there's an indentation issue with the closing bracket.Apply this diff to fix the indentation:
named_sinks=named_sinks, - ) + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
robusta_krr/main.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
robusta_krr/main.py (1)
robusta_krr/core/runner.py (1)
publish_input_error(527-528)
🪛 Ruff (0.12.2)
robusta_krr/main.py
311-316: Do not perform function call typer.Option in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🪛 Flake8 (7.2.0)
robusta_krr/main.py
[error] 369-369: closing bracket does not match indentation of opening bracket's line
(E123)
🔇 Additional comments (2)
robusta_krr/main.py (2)
311-316: LGTM! Parameter definition follows established patterns.The
named_sinksparameter is correctly defined with appropriate type annotation, help text, and panel grouping, consistent with other CLI parameters in the function.Note: The static analysis B008 warning about function calls in argument defaults is a false positive here, as
typer.Option()is the standard pattern for typer CLI parameters.
373-373: LGTM! Function call parameter order is now correct.The
publish_input_errorfunction call now has the correct parameter order matching the function signature:(url, scan_id, start_time, error, named_sinks).
arikalon1
left a comment
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.
nice work
No description provided.