-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
fix(django): catch the right error when trying to close db connection #9392
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9392 +/- ##
==========================================
+ Coverage 78.15% 78.17% +0.01%
==========================================
Files 153 153
Lines 19029 19030 +1
Branches 2518 2518
==========================================
+ Hits 14873 14876 +3
+ Misses 3867 3863 -4
- Partials 289 291 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I don't really understand why Codecov is complaining, as the line I added is tested. And why are all the import lines considered as not tested ? |
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.
can you please increase test coverage for the proposed change please?
I'll try to improve the coverage, but I don't get why codecov is complaining, thz line I added is tested, and the only line it's highligting is the function definition |
@auvipy I honestly don't know how to improve the coverage, by looking at the report, only the function signatures are not covered. Isn't there a problem with the codecov configuration ? |
@auvipy it's seems related to this pytest-cov issue. |
Hey @auvipy do you have any news about this ? Are you waiting for the v5.5.0 to be released ? |
e7eab0c
to
ed6708c
Compare
Description
Fixes #9310
As explained in the issue,
_maybe_close_db_fd
tries to catch django's db-agnostic errors (such asdjango.db.InterfaceError
), but_maybe_close_fd
raises a db-specific error (eg.pyscog2.InterfaceError
).To fix that, we wrap the call to
_maybe_close_fd
with thec.wrap_database_errors
context manager (cf here), made for this exact purpose. It prevents the worker to keep raising errors when initializing a processThis can easily be reproduced: