-
Notifications
You must be signed in to change notification settings - Fork 153
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
feature/post-prototype-client #640
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in the pull request primarily focus on enhancing the functionality of the Changes
Poem
🪧 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 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
🧹 Outside diff range and nitpick comments (4)
safety/scan/main.py (4)
230-231
: Specify file encoding when reading filesWhen reading files, it's good practice to specify the encoding to avoid potential issues with files that may not be UTF-8 encoded.
Apply this diff to specify UTF-8 encoding:
- with open(f_path, "r") as file: + with open(f_path, "r", encoding="utf-8") as file:
249-250
: Use logging instead of print statements for consistent loggingUsing the logging module instead of print statements ensures consistency and better control over log levels and outputs.
Apply this diff to use logging:
- print("Prepared files_metadata payload for API POST request:") - print(files_metadata) + LOG.debug("Prepared files_metadata payload for API POST request:") + LOG.debug(files_metadata)
260-260
: Fix typo in log messageThere's a typo in the log message: "Sccan" should be "Scan".
Apply this diff to correct the typo:
- LOG.info("Sccan Payload successfully sent to the API.") + LOG.info("Scan payload successfully sent to the API.")
253-266
: Enhance error handling for API requestsCurrently, the code logs errors but does not handle unsuccessful responses or retry failed requests. Consider adding retries for transient network errors and handling different HTTP status codes appropriately.
Consider implementing:
- Retries with exponential backoff for transient errors using a library like
tenacity
orrequests.adapters.HTTPAdapter
.- Detailed handling of HTTP status codes, especially for client errors (4xx) and server errors (5xx), to provide more informative error messages or take corrective actions.
- Timeouts for the API request to prevent the application from hanging indefinitely.
Example using
requests
with a timeout and retry mechanism:import requests from requests.adapters import HTTPAdapter, Retry # Setup retries for the session session = requests.Session() retries = Retry(total=3, backoff_factor=0.3, status_forcelist=[500, 502, 503, 504]) adapter = HTTPAdapter(max_retries=retries) session.mount('https://', adapter) session.mount('http://', adapter) # Send the request with a timeout try: response = session.post( SCAN_API_ENDPOINT, json={"files_metadata": files_metadata}, headers=headers, timeout=10 # seconds ) response.raise_for_status() LOG.info("Scan payload successfully sent to the API.") except requests.exceptions.HTTPError as http_err: LOG.error(f"HTTP error occurred: {http_err}") except requests.exceptions.Timeout: LOG.error("The request timed out") except requests.exceptions.RequestException as err: LOG.error(f"An error occurred: {err}")
0b31405
to
223ad60
Compare
c23978b
to
c757b65
Compare
6e4fad4
to
76639d2
Compare
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.
LGTM
bcc5b85
to
ecab918
Compare
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.
LGTM!
9b848a1
to
c7f13dd
Compare
c7f13dd
to
293a6df
Compare
6fa375f
to
ae35cce
Compare
Description
This PR introduces a new flag, --use-server-matching, to the scan command. The flag enables using server-side vulnerability matching, allowing the scan command to send file metadata to the server for processing. This represents the initial step in transitioning from the old GET-based implementation to a POST-based server interaction.
Behavior:
Key updates:
Purpose:
This change lays the foundation for a complete migration to server-side vulnerability matching, enabling more scalable and centralized file processing.