-
Notifications
You must be signed in to change notification settings - Fork 420
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
ci: Migrate to ruff
#5150
ci: Migrate to ruff
#5150
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
for more information, see https://pre-commit.ci
Docker builds report
|
@@ -71,14 +70,14 @@ | |||
return func(*args, **kwargs) # type: ignore[no-any-return] | |||
except ValueError as e: | |||
return Response( | |||
data={"detail": (f"{error or default_error}" f" Error: {str(e)}")}, | |||
data={"detail": (f"{error or default_error} Error: {str(e)}")}, |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Stack trace information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 3 days ago
To fix the problem, we need to ensure that detailed error messages are not exposed to the end user. Instead, we should log the detailed error message on the server and return a generic error message to the user. This can be achieved by modifying the github_api_call_error_handler
decorator to log the exception details and return a generic error message.
- Modify the
github_api_call_error_handler
decorator to log the exception details using thelogger
and return a generic error message to the user. - Ensure that the logging captures the necessary details for debugging without exposing them to the user.
-
Copy modified line R72 -
Copy modified line R74 -
Copy modified line R81
@@ -71,4 +71,5 @@ | ||
except ValueError as e: | ||
logger.error(f"{error or default_error} Error: {str(e)}", exc_info=e) | ||
return Response( | ||
data={"detail": (f"{error or default_error} Error: {str(e)}")}, | ||
data={"detail": default_error}, | ||
content_type="application/json", | ||
@@ -79,3 +80,3 @@ | ||
return Response( | ||
data={"detail": (f"{error or default_error} Error: {str(e)}")}, | ||
data={"detail": default_error}, | ||
content_type="application/json", |
content_type="application/json", | ||
status=status.HTTP_400_BAD_REQUEST, | ||
) | ||
except requests.RequestException as e: | ||
logger.error(f"{error or default_error} Error: {str(e)}", exc_info=e) | ||
return Response( | ||
data={"detail": (f"{error or default_error}" f" Error: {str(e)}")}, | ||
data={"detail": (f"{error or default_error} Error: {str(e)}")}, |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Stack trace information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 3 days ago
To fix the problem, we need to ensure that detailed error messages, including stack traces, are not exposed to external users. Instead, we should log the detailed error message on the server and return a generic error message to the user. This can be achieved by modifying the github_api_call_error_handler
decorator to log the exception details and return a generic error message.
- Modify the
github_api_call_error_handler
decorator to log the detailed error message using thelogger
and return a generic error message to the user. - Ensure that the logging includes the stack trace for debugging purposes.
-
Copy modified line R80
@@ -79,3 +79,3 @@ | ||
return Response( | ||
data={"detail": (f"{error or default_error} Error: {str(e)}")}, | ||
data={"detail": (f"{error or default_error} Error occurred while processing your request.")}, | ||
content_type="application/json", |
Uffizzi Preview |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5150 +/- ##
=======================================
Coverage 97.46% 97.47%
=======================================
Files 1224 1224
Lines 42573 42567 -6
=======================================
- Hits 41495 41491 -4
+ Misses 1078 1076 -2 ☔ View full report in Codecov by Sentry. |
Thanks for submitting a PR! Please check the boxes below:
docs/
if required so people know about the feature!Changes
This PR migrates all of Python linting and formatting, including import sorting, to
ruff
.I tried my best to preserve original linting rules and formatting configuration, however, since the latter was inconsistent across
black
andisort
, there's still a hefty load of changes that come along with the migration.The initial rationale behind this to restore import order linting after we had to turn it off to be able to merge #5119. I have confirmed that
ruff
does not force incorrecttype: ignore
comment placement, unlikeisort
(see PyCQA/isort#2244).The overall developer experience and performance are resoundingly better that the
flake8
-black
-isort
combination we had to support earlier.Once this is merged, I'll open a follow-up PR adding the formatting changes to
.git-blame-ignore-revs
.How did you test this code?
Ran
pre-commit
andruff check
/ruff format
, making sure the outcomes are consistent.