Skip to content

Conversation

@Avi-Robusta
Copy link
Contributor

@Avi-Robusta Avi-Robusta commented Jun 26, 2025

tested in cluster and cli
in cluster verified errors in multiple places in code are sent to scan failure in the db

@coderabbitai
Copy link

coderabbitai bot commented Jun 26, 2025

"""

Walkthrough

Three new optional configuration fields—publish_scan_url, start_time, and scan_id—were added to support publishing scan results and errors to an external HTTP endpoint. The CLI, configuration model, and runner logic were updated to accept these fields, send results and errors via HTTP POST with retry logic, and log the process. Error publishing is integrated into validation and runtime error handling.

Changes

File(s) Change Summary
robusta_krr/core/models/config.py Added optional fields publish_scan_url, start_time, and scan_id to the Config class.
robusta_krr/core/runner.py Added methods to send scan results and errors as JSON payloads via HTTP POST with retries; integrated calls into error handling and result processing.
robusta_krr/main.py Added CLI options for publishing scan settings; integrated error publishing on validation failure in argument parsing.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI (main.py)
    participant Config
    participant Runner
    participant HTTP Endpoint

    User->>CLI (main.py): run_strategy with options (publish_scan_url, scan_id, start_time)
    CLI (main.py)->>Config: Construct Config object
    alt ValidationError
        CLI (main.py)->>Runner: publish_input_error(publish_scan_url, scan_id, start_time, error)
        Runner->>HTTP Endpoint: POST error payload
    else Valid Config
        CLI (main.py)->>Runner: run(Config)
        Runner->>Kubernetes: Load configuration
        alt Error loading configuration
            Runner->>Runner: publish_error(publish_scan_url, scan_id, start_time, error)
            Runner->>HTTP Endpoint: POST error payload
        else Success
            Runner->>Runner: _process_result()
            Runner->>Runner: _send_result(publish_scan_url, start_time, scan_id, result)
            Runner->>HTTP Endpoint: POST result payload
        end
    end
Loading

"""

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
robusta_krr/core/models/config.py (1)

63-66: Consider adding validation for the new publishing fields.

The new optional fields are well-structured. Consider adding validators similar to the existing prometheus_url validator to ensure:

  • publish_scan_url has proper URL format
  • start_time has valid datetime format
  • scan_id has valid UUID format (if expected)

This would provide better error messages and catch configuration issues early.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee37fba and 32e4f04.

📒 Files selected for processing (4)
  • robusta_krr/core/models/config.py (1 hunks)
  • robusta_krr/core/runner.py (4 hunks)
  • robusta_krr/formatters/json.py (1 hunks)
  • robusta_krr/main.py (3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
robusta_krr/core/runner.py (2)
robusta_krr/formatters/json.py (1)
  • json (6-7)
robusta_krr/core/models/result.py (1)
  • Result (62-115)
🪛 Flake8 (7.2.0)
robusta_krr/main.py

[error] 334-334: closing bracket does not match indentation of opening bracket's line

(E123)


[error] 338-338: whitespace after '('

(E201)

robusta_krr/core/runner.py

[error] 110-110: missing whitespace after keyword

(E275)


[error] 370-370: expected 2 blank lines, found 1

(E302)


[error] 373-373: redefinition of unused 'publish_error' from line 370

(F811)


[error] 373-373: expected 2 blank lines, found 1

(E302)


[error] 378-378: expected 2 blank lines, found 1

(E302)

🪛 Ruff (0.11.9)
robusta_krr/core/runner.py

373-373: Redefinition of unused publish_error from line 370

(F811)

🪛 Pylint (3.3.7)
robusta_krr/core/runner.py

[error] 373-373: function already defined line 370

(E0102)

🔇 Additional comments (5)
robusta_krr/formatters/json.py (1)

7-7: LGTM! Minor formatting improvement.

Removing the trailing newline is a good formatting practice.

robusta_krr/main.py (1)

269-286: LGTM! Well-structured CLI options for publishing settings.

The new CLI options are properly organized under the "Publish Scan Settings" help panel and match the configuration fields.

robusta_krr/core/runner.py (3)

13-15: LGTM! Required imports for HTTP functionality.

The new imports support the HTTP publishing functionality and error handling.


337-337: LGTM! Appropriate error publishing in exception handlers.

The error publishing calls are well-placed in the exception handling blocks to capture and report failures.

Also applies to: 357-357, 361-361


366-368: LGTM! Clean result publishing method.

The _send_result method properly converts the Result to JSON and delegates to the helper function.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
robusta_krr/core/runner.py (2)

369-373: Fix critical function redefinition issue.

There are two functions with the same name publish_error, causing a redefinition error. The second function should have a different name.

Apply this diff to fix the redefinition:

 def publish_error(url: str, scan_id: str, start_time: str, error: str):
     _send_scan_payload(url, scan_id, start_time, error, is_error=True)

-def publish_error(error: str):
+
+def publish_error_from_settings(error: str):
+    if not settings.publish_scan_url or not settings.scan_id or not settings.start_time:
+        return
     _send_scan_payload(settings.publish_scan_url, settings.scan_id, settings.start_time, error, is_error=True)

You'll also need to update the calls at lines 336, 356, and 360 to use publish_error_from_settings.


375-416: Improve error handling and fix formatting issues.

The function implementation is solid but needs formatting fixes and enhanced error handling as previously suggested.

Apply this diff to address the issues:

+
 def _send_scan_payload(
     url: str,
     scan_id: str,
     start_time: Union[str, datetime],
     result_data: Union[str, dict],
     is_error: bool = False
 ):
     if not url or not scan_id or not start_time:
         return

     headers = {"Content-Type": "application/json"}

     if isinstance(start_time, datetime):
         start_time = start_time.isoformat()

     action_request = {
         "action_name": "process_scan",
         "action_params": {
             "result": result_data,
             "scan_type": "krr",
             "scan_id": scan_id,
             "start_time": start_time,
         }
     }

     try:
         logger_msg = "Sending error scan" if is_error else "Sending scan"
-        logger.warning(logger_msg)
+        logger.info(logger_msg)

         response = requests.post(
             url,
             headers=headers,
-            json=action_request  # Let requests handle encoding
+            json=action_request,
+            timeout=30  # Add timeout to prevent hanging
         )

+        response.raise_for_status()  # Raise exception for HTTP errors
         logger.info(f"Status code: {response.status_code}")
         logger.info(f"Response: {response.text}")

     except requests.exceptions.RequestException:
         logger.error("HTTP RequestException", exc_info=True)
     except Exception:
         logger.error("Unexpected exception while sending scan result", exc_info=True)
🧹 Nitpick comments (1)
robusta_krr/core/runner.py (1)

365-367: Optimize JSON conversion to avoid unnecessary serialization.

The current implementation serializes to JSON string then deserializes back to dict, which is inefficient.

    def _send_result(self, url: str, start_time: datetime, scan_id: str, result: Result):
-        result_dict = json.loads(result.json(indent=2))
+        result_dict = result.dict()
        _send_scan_payload(url, scan_id, start_time, result_dict, is_error=False)

This avoids the unnecessary JSON string conversion and uses the Pydantic model's dict() method directly.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d427c05 and 2a9f93d.

📒 Files selected for processing (1)
  • robusta_krr/core/runner.py (4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
robusta_krr/core/runner.py (2)
robusta_krr/formatters/json.py (1)
  • json (6-7)
robusta_krr/core/models/result.py (1)
  • Result (62-115)
🪛 Ruff (0.11.9)
robusta_krr/core/runner.py

372-372: Redefinition of unused publish_error from line 369

(F811)

🪛 Flake8 (7.2.0)
robusta_krr/core/runner.py

[error] 369-369: expected 2 blank lines, found 1

(E302)


[error] 372-372: redefinition of unused 'publish_error' from line 369

(F811)


[error] 372-372: expected 2 blank lines, found 1

(E302)


[error] 375-375: expected 2 blank lines, found 1

(E302)

🪛 Pylint (3.3.7)
robusta_krr/core/runner.py

[error] 372-372: function already defined line 369

(E0102)

🔇 Additional comments (2)
robusta_krr/core/runner.py (2)

13-15: LGTM - Appropriate imports for new HTTP publishing functionality.

The new imports (requests, json, traceback) are necessary and correctly used for the HTTP publishing capabilities.


336-336: Error publishing calls are correct but depend on resolving function redefinition.

The publish_error calls are appropriately placed in exception handlers. However, they depend on resolving the function redefinition issue below.

Also applies to: 356-356, 360-360

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (2)
robusta_krr/main.py (1)

331-334: Fix formatting issue with closing parenthesis.

The closing parenthesis indentation needs to be corrected to align with the opening bracket.

Apply this diff to fix the formatting:

                        publish_scan_url=publish_scan_url,
                        start_time=start_time,
                        scan_id=scan_id,
-                        )
+                    )
robusta_krr/core/runner.py (1)

111-111: Add conditional check before sending results.

The _send_result call should be conditional to avoid unnecessary processing when publishing is not configured.

Apply this diff to add the conditional check:

-        self._send_result(settings.publish_scan_url, settings.start_time, settings.scan_id, result)
+        if settings.publish_scan_url and settings.start_time and settings.scan_id:
+            self._send_result(settings.publish_scan_url, settings.start_time, settings.scan_id, result)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e49e597 and 0805a23.

📒 Files selected for processing (2)
  • robusta_krr/core/runner.py (4 hunks)
  • robusta_krr/main.py (3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
robusta_krr/core/runner.py (2)
robusta_krr/formatters/json.py (1)
  • json (6-7)
robusta_krr/core/models/result.py (1)
  • Result (62-115)
🪛 Pylint (3.3.7)
robusta_krr/core/runner.py

[error] 337-337: No value for argument 'scan_id' in function call

(E1120)


[error] 337-337: No value for argument 'start_time' in function call

(E1120)


[error] 337-337: No value for argument 'error' in function call

(E1120)


[error] 357-357: No value for argument 'scan_id' in function call

(E1120)


[error] 357-357: No value for argument 'start_time' in function call

(E1120)


[error] 357-357: No value for argument 'error' in function call

(E1120)


[error] 361-361: No value for argument 'scan_id' in function call

(E1120)


[error] 361-361: No value for argument 'start_time' in function call

(E1120)


[error] 361-361: No value for argument 'error' in function call

(E1120)

robusta_krr/main.py

[error] 338-338: Too many positional arguments for function call

(E1121)

🪛 Flake8 (7.2.0)
robusta_krr/core/runner.py

[error] 370-370: expected 2 blank lines, found 1

(E302)


[error] 373-373: expected 2 blank lines, found 1

(E302)


[error] 376-376: expected 2 blank lines, found 1

(E302)

robusta_krr/main.py

[error] 334-334: closing bracket does not match indentation of opening bracket's line

(E123)


[error] 338-338: whitespace after '('

(E201)

🔇 Additional comments (5)
robusta_krr/main.py (2)

20-20: LGTM: Import addition supports the new error publishing functionality.

The import of publish_input_error is correctly added to support validation error publishing.


269-286: LGTM: CLI options properly implement the publishing scan settings.

The three new CLI options are well-defined with appropriate help text and are correctly grouped under the "Publish Scan Settings" panel.

robusta_krr/core/runner.py (3)

6-6: LGTM: Import additions support the new HTTP publishing functionality.

The new imports (time, requests, json, traceback) are correctly added to support the retry logic, HTTP communication, JSON serialization, and error formatting capabilities.

Also applies to: 14-16


366-368: LGTM: Result sending method is well-implemented.

The _send_result method correctly converts the Result object to JSON and delegates to the payload sending function.


376-434: LGTM: Robust HTTP payload sending with retry logic.

The _send_scan_payload function is well-implemented with:

  • Proper parameter validation
  • Comprehensive logging with scan_id context
  • Exponential backoff retry mechanism
  • Appropriate error handling for different exception types

The implementation aligns well with the PR objective of ensuring reliable result publishing.

@Avi-Robusta Avi-Robusta requested a review from moshemorad June 29, 2025 10:45
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
robusta_krr/core/runner.py (2)

111-111: Add conditional check before sending results.

The result sending should be conditional to avoid unnecessary processing when publishing parameters are not configured.

-        self._send_result(settings.publish_scan_url, settings.start_time, settings.scan_id, result)
+        if settings.publish_scan_url and settings.start_time and settings.scan_id:
+            self._send_result(settings.publish_scan_url, settings.start_time, settings.scan_id, result)

376-433: Consider using tenacity library for retry logic.

While the manual retry implementation with exponential backoff is functional, using the tenacity library would provide more robust and maintainable retry logic as previously suggested.

+from tenacity import retry, stop_after_attempt, wait_exponential, retry_if_exception_type

+@retry(
+    stop=stop_after_attempt(3),
+    wait=wait_exponential(multiplier=1, min=1, max=10),
+    retry=retry_if_exception_type(requests.exceptions.RequestException)
+)
+def _send_scan_payload_with_retry(url: str, headers: dict, action_request: dict, scan_id: str, is_error: bool):
+    logger_msg = "Sending error scan" if is_error else "Sending scan"
+    logger.info(f"{logger_msg} for scan_id={scan_id} to url={url}")
+    
+    response = requests.post(url, headers=headers, json=action_request, timeout=30)
+    response.raise_for_status()
+    
+    logger.info(f"scan_id={scan_id} | Status code: {response.status_code}")
+    logger.info(f"scan_id={scan_id} | Response body: {response.text}")
+    return response

 def _send_scan_payload(
     url: str,
     scan_id: str,
     start_time: Union[str, datetime],
     result_data: Union[str, dict],
     is_error: bool = False
 ):
     if not url or not scan_id or not start_time:
         logger.debug(f"Missing required parameters: url={bool(url)}, scan_id={bool(scan_id)}, start_time={bool(start_time)}")
         return

     logger.debug(f"Preparing to send scan payload. scan_id={scan_id}, is_error={is_error}")

     headers = {"Content-Type": "application/json"}

     if isinstance(start_time, datetime):
         logger.debug(f"Converting datetime to ISO format for scan_id={scan_id}")
         start_time = start_time.isoformat()

     action_request = {
         "action_name": "process_scan",
         "action_params": {
             "result": result_data,
             "scan_type": "krr",
             "scan_id": scan_id,
             "start_time": start_time,
         }
     }

-    retries = 3
-    backoff = 1
-
-    for attempt in range(1, retries + 1):
-        try:
-            logger_msg = "Sending error scan" if is_error else "Sending scan"
-            logger.info(f"{logger_msg} for scan_id={scan_id} to url={url} (attempt {attempt})")
-
-            response = requests.post(
-                url,
-                headers=headers,
-                json=action_request  # Let requests handle encoding
-            )
-
-            logger.info(f"scan_id={scan_id} | Status code: {response.status_code}")
-            logger.info(f"scan_id={scan_id} | Response body: {response.text}")
-            break  # Success, exit retry loop
-
-        except requests.exceptions.RequestException as e:
-            logger.warning(f"scan_id={scan_id} | Attempt {attempt} failed with RequestException: {e}", exc_info=True)
-        except Exception as e:
-            logger.error(f"scan_id={scan_id} | Attempt {attempt} failed with unexpected exception: {e}", exc_info=True)
-
-        if attempt < retries:
-            logger.info(f"scan_id={scan_id} | Retrying in {backoff} seconds...")
-            time.sleep(backoff)
-            backoff *= 2  # Exponential backoff
-        else:
-            logger.error(f"scan_id={scan_id} | All retry attempts failed.")
+    try:
+        _send_scan_payload_with_retry(url, headers, action_request, scan_id, is_error)
+    except Exception as e:
+        logger.error(f"scan_id={scan_id} | All retry attempts failed: {e}", exc_info=True)
🧹 Nitpick comments (2)
robusta_krr/core/runner.py (2)

370-371: Fix formatting issues with blank lines.

Static analysis correctly identifies missing blank lines before function definitions.

         return 0  # Exit with success

+
 def publish_input_error(url: str, scan_id: str, start_time: str, error: str):
     _send_scan_payload(url, scan_id, start_time, error, is_error=True)

+
 def publish_error(error: str):
     _send_scan_payload(settings.publish_scan_url, settings.scan_id, settings.start_time, error, is_error=True)

Also applies to: 373-374


376-433: Add missing blank lines and improve error handling.

The function implements good retry logic and logging, but needs formatting fixes and could benefit from timeout handling.

     _send_scan_payload(settings.publish_scan_url, settings.scan_id, settings.start_time, error, is_error=True)

+
 def _send_scan_payload(
     url: str,
     scan_id: str,
     start_time: Union[str, datetime],
     result_data: Union[str, dict],
     is_error: bool = False
 ):

Also consider adding a timeout to the requests.post call:

             response = requests.post(
                 url,
                 headers=headers,
-                json=action_request  # Let requests handle encoding
+                json=action_request,  # Let requests handle encoding
+                timeout=30  # Add timeout to prevent hanging
             )
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0805a23 and 1862e62.

📒 Files selected for processing (1)
  • robusta_krr/core/runner.py (4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
robusta_krr/core/runner.py (2)
robusta_krr/formatters/json.py (1)
  • json (6-7)
robusta_krr/core/models/result.py (1)
  • Result (62-115)
🪛 Flake8 (7.2.0)
robusta_krr/core/runner.py

[error] 370-370: expected 2 blank lines, found 1

(E302)


[error] 373-373: expected 2 blank lines, found 1

(E302)


[error] 376-376: expected 2 blank lines, found 1

(E302)

🔇 Additional comments (2)
robusta_krr/core/runner.py (2)

6-6: LGTM! New imports are appropriate.

The new imports (time, requests, json, traceback) are correctly added to support the HTTP publishing functionality with retry logic and error handling.

Also applies to: 14-16


337-337: LGTM! Function calls are now correctly implemented.

The publish_error calls are now properly implemented to use the global settings, resolving the previous missing arguments issue.

Also applies to: 357-357, 361-361

Copy link

@coderabbitai coderabbitai bot left a 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 (2)
robusta_krr/core/runner.py (2)

112-112: Make result sending conditional.

The _send_result call should be conditional to avoid unnecessary processing when publishing is not configured.

-        self._send_result(settings.publish_scan_url, settings.start_time, settings.scan_id, result)
+        if settings.publish_scan_url and settings.start_time and settings.scan_id:
+            self._send_result(settings.publish_scan_url, settings.start_time, settings.scan_id, result)

367-369: Add error handling to prevent failures from breaking the main flow.

The method should handle potential exceptions during JSON serialization and HTTP sending.

 def _send_result(self, url: str, start_time: datetime, scan_id: str, result: Result):
+    if not url or not scan_id or not start_time:
+        logger.debug("Missing required parameters for result publishing")
+        return
+    
+    try:
         result_dict = json.loads(result.json(indent=2))
         _send_scan_payload(url, scan_id, start_time, result_dict, is_error=False)
+    except Exception as e:
+        logger.error(f"Failed to send result for scan_id={scan_id}: {e}", exc_info=True)
🧹 Nitpick comments (2)
robusta_krr/core/runner.py (2)

6-6: Remove unused import.

The time import is not used anywhere in the file and should be removed.

-import time

371-371: Fix formatting: add missing blank lines before function definitions.

Python PEP8 requires 2 blank lines before top-level function definitions.

         else:
             return 0  # Exit with success

+
 def publish_input_error(url: str, scan_id: str, start_time: str, error: str):
     _send_scan_payload(url, scan_id, start_time, error, is_error=True)

+
 def publish_error(error: str):
     _send_scan_payload(settings.publish_scan_url, settings.scan_id, settings.start_time, error, is_error=True)

+
 @retry(
     stop=stop_after_attempt(3),
     wait=wait_exponential(multiplier=1, min=1, max=8),
     retry=retry_if_exception_type(requests.exceptions.RequestException),
     reraise=True
 )

Also applies to: 374-374, 377-377

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1862e62 and fcd7db5.

📒 Files selected for processing (1)
  • robusta_krr/core/runner.py (4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
robusta_krr/core/runner.py (2)
robusta_krr/formatters/json.py (1)
  • json (6-7)
robusta_krr/core/models/result.py (1)
  • Result (62-115)
🪛 Ruff (0.11.9)
robusta_krr/core/runner.py

6-6: time imported but unused

Remove unused import: time

(F401)

🪛 Flake8 (7.2.0)
robusta_krr/core/runner.py

[error] 6-6: 'time' imported but unused

(F401)


[error] 371-371: expected 2 blank lines, found 1

(E302)


[error] 374-374: expected 2 blank lines, found 1

(E302)


[error] 377-377: expected 2 blank lines, found 1

(E302)

🔇 Additional comments (3)
robusta_krr/core/runner.py (3)

338-338: Error publishing implementation looks correct.

The publish_error calls are properly integrated into the exception handling flow with appropriate error messages.

Also applies to: 358-358, 362-362


377-390: Excellent retry implementation using tenacity.

The retry mechanism is well-implemented with proper exponential backoff, specific exception handling, and comprehensive logging including scan_id for monitoring. This addresses the previous feedback about using tenacity instead of manual retry implementation.


393-427: Well-implemented payload sending function.

The function includes comprehensive parameter validation, proper datetime handling, structured JSON payload construction, and excellent error handling with detailed logging. The separation of concerns between payload construction and HTTP request execution is well-designed.

Copy link

@coderabbitai coderabbitai bot left a 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 (2)
robusta_krr/core/runner.py (2)

6-6: Remove unused imports.

Static analysis correctly identifies unused imports that should be removed.

Apply this diff to remove unused imports:

-import time
-from tenacity import retry, stop_after_attempt, wait_exponential, retry_if_exception_type
+from tenacity import retry, stop_after_attempt, wait_exponential

Also applies to: 14-14


371-372: Fix formatting issues with function definitions.

Missing blank lines before function definitions violate PEP8 standards.

Apply this diff to add required blank lines:

         else:
             return 0  # Exit with success

+
 def publish_input_error(url: str, scan_id: str, start_time: str, error: str):
     _send_scan_payload(url, scan_id, start_time, error, is_error=True)

+
 def publish_error(error: str):
     _send_scan_payload(settings.publish_scan_url, settings.scan_id, settings.start_time, error, is_error=True)

Also applies to: 374-375

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fcd7db5 and 5d28be1.

📒 Files selected for processing (3)
  • robusta_krr/core/models/config.py (1 hunks)
  • robusta_krr/core/runner.py (4 hunks)
  • robusta_krr/main.py (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • robusta_krr/core/models/config.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
robusta_krr/core/runner.py (2)
robusta_krr/formatters/json.py (1)
  • json (6-7)
robusta_krr/core/models/result.py (1)
  • Result (62-115)
robusta_krr/main.py (2)
robusta_krr/core/runner.py (2)
  • Runner (44-369)
  • publish_input_error (371-372)
robusta_krr/core/models/config.py (1)
  • Config (20-187)
🪛 Ruff (0.11.9)
robusta_krr/core/runner.py

6-6: time imported but unused

Remove unused import: time

(F401)


14-14: tenacity.retry_if_exception_type imported but unused

Remove unused import: tenacity.retry_if_exception_type

(F401)

🪛 Flake8 (7.2.0)
robusta_krr/core/runner.py

[error] 6-6: 'time' imported but unused

(F401)


[error] 14-14: 'tenacity.retry_if_exception_type' imported but unused

(F401)


[error] 371-371: expected 2 blank lines, found 1

(E302)


[error] 374-374: expected 2 blank lines, found 1

(E302)


[error] 377-377: expected 2 blank lines, found 1

(E302)

robusta_krr/main.py

[error] 334-334: closing bracket does not match indentation of opening bracket's line

(E123)


[error] 338-338: whitespace after '('

(E201)

🔇 Additional comments (8)
robusta_krr/main.py (3)

269-286: LGTM! New CLI parameters are well-defined.

The three new CLI parameters for publish scan settings are properly defined with appropriate help text and grouped under a logical help panel.


331-334: LGTM! Config constructor properly updated.

The new parameters are correctly added to the Config constructor call with proper indentation.


338-338: Fix parameter order and formatting in function call.

The parameters are passed in the wrong order and there's a formatting issue with extra space after the opening parenthesis.

Apply this diff to fix the parameter order and formatting:

-                    publish_input_error( publish_scan_url, start_time, scan_id, str(e))
+                    publish_input_error(publish_scan_url, scan_id, start_time, str(e))

The function expects: (url, scan_id, start_time, error) but the call passes: (url, start_time, scan_id, error).

Likely an incorrect or invalid review comment.

robusta_krr/core/runner.py (5)

112-112: LGTM! Result sending integration is appropriate.

The call to _send_result is correctly placed after result processing, and the _send_scan_payload function properly validates required parameters before proceeding.


338-338: LGTM! Error publishing calls are correct.

The publish_error function calls are properly implemented. The function correctly uses global settings internally to access the required URL, scan_id, and start_time parameters.

Also applies to: 358-358, 362-362


367-369: LGTM! Result sending implementation is solid.

The _send_result method correctly converts the Result object to JSON and delegates to the payload sending function.


377-388: LGTM! Retry mechanism is well-implemented.

The retry decorator with exponential backoff is appropriately configured for HTTP requests. The logging includes scan_id for traceability as requested in past reviews.


391-425: LGTM! Payload sending logic is robust.

The _send_scan_payload function includes proper parameter validation, datetime conversion, structured JSON payload construction, and comprehensive error handling with detailed logging that includes scan_id for monitoring.

@Avi-Robusta Avi-Robusta merged commit b96bfdd into main Jul 6, 2025
3 checks passed
@Avi-Robusta Avi-Robusta deleted the push_results branch July 6, 2025 14:25
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.

3 participants