-
Notifications
You must be signed in to change notification settings - Fork 150
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
feat(scan): include exit code and reason in JSON result #555
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe recent changes enhance the system's error handling and reporting capabilities by introducing a new exit code mapping dictionary. This addition clarifies the context of exit codes and improves the reporting function by incorporating exit status and reasons into JSON outputs. Overall, these modifications bolster user feedback and make the codebase more maintainable and informative. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add 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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- safety/constants.py (1 hunks)
- safety/scan/command.py (32 hunks)
Additional context used
Ruff
safety/scan/command.py
472-472: Local variable
fixes
is assigned to but never usedRemove assignment to unused variable
fixes
(F841)
Additional comments not posted (6)
safety/constants.py (1)
166-178
: Addition ofEXIT_CODE_REASON_MAPPING
is well-structured.The new dictionary
EXIT_CODE_REASON_MAPPING
provides clear and descriptive messages for each exit code, enhancing the readability and maintainability of the codebase.safety/scan/command.py (5)
52-53
: New parametersexit_code
andexit_reason
are added toprocess_report
.The function signature now includes
exit_code
andexit_reason
, which are essential for enhancing the reporting capabilities.
57-60
: Inclusion ofexit_code
andexit_reason
in JSON output.The
exit_code
andexit_reason
are correctly added to the JSON output, providing more detailed information about the scan results.
437-439
: Determination ofexit_reason
based onexit_code
.The
exit_reason
is correctly determined usingEXIT_CODE_REASON_MAPPING
, ensuring that a descriptive reason is provided for each exit code.
439-439
: Updated call toprocess_report
includesexit_code
andexit_reason
.The call to
process_report
now correctly includes theexit_code
andexit_reason
, enhancing the reporting functionality.
760-760
: Updated call toprocess_report
insystem_scan
includes default values.The call to
process_report
now includes the default values forexit_code
andexit_reason
, ensuring consistency in the reporting functionality.
safety/scan/command.py
Outdated
fixes = process_fixes_scan(file_to_fix, | ||
|
||
fixes = process_fixes_scan(file_to_fix, |
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.
Remove unused variable fixes
.
The variable fixes
is assigned but never used, which is unnecessary.
- fixes = process_fixes_scan(file_to_fix,
- specs_to_fix, update_limits, output, no_output=no_output,
- prompt=prompt)
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.
fixes = process_fixes_scan(file_to_fix, | |
process_fixes_scan(file_to_fix, | |
specs_to_fix, update_limits, output, no_output=no_output, | |
prompt=prompt) |
Tools
Ruff
472-472: Local variable
fixes
is assigned to but never usedRemove assignment to unused variable
fixes
(F841)
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.
i agree with coderabbit's suggested change here: remove fixes =
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.
"""
@jeff-at-safety, thanks for confirming the change. I'll proceed to remove the assignment to the fixes
variable as suggested.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
I noticed that a datetime serialization error is thrown during testing. I don't believe this issue is directly related to the changes introduced here, but we could introduce datetime serialization logic to address it if needed. Specifically, we can add a custom serializer to handle datetime objects when generating the JSON result. Please let me know if you would like me to include this fix in the current PR! |
safety/scan/command.py
Outdated
fixes = process_fixes_scan(file_to_fix, | ||
|
||
fixes = process_fixes_scan(file_to_fix, |
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.
i agree with coderabbit's suggested change here: remove fixes =
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- safety/scan/command.py (32 hunks)
Files skipped from review as they are similar to previous changes (1)
- safety/scan/command.py
Description:
This PR introduces enhancements to the
scan
andprocess_report
functions to include the exit code and corresponding reason in the JSON result. This improvement aims to provide better insights into the scan outcomes, making it easier for the platform to understand and act upon the results. This also ensures that the exit code result will be in audit logs for historical data and for customers that ingest and parse and rely on this JSON output.Changes Made:
exit_code
andexit_reason
with default values (0
and"No vulnerabilities found."
respectively).EXIT_CODE_REASON_MAPPING
to map exit codes to their corresponding reasons.exit_reason
based on the determinedexit_code
using the mapping dictionary.exit_code
andexit_reason
.Details:
EXIT_CODE_REASON_MAPPING
insafety/constants.py
to map predefined exit codes to user-friendly reason strings. This helps in providing clear and understandable reasons for each exit code.process_report
function now accepts and includesexit_code
andexit_reason
in the generated JSON report. This allows for more comprehensive reporting of scan results.scan
function to determine and setexit_reason
based on theexit_code
. Similarly, ensured thesystem_scan
function passes default values for these parameters toprocess_report
.Rationale:
Including the exit code and its corresponding reason in the JSON result enhances the clarity of scan reports. This improvement facilitates better decision-making and error handling on the platform, ultimately improving the user experience.
Summary by CodeRabbit
New Features
Bug Fixes
Style