-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Replace OSError exceptions from can_read
with redis.ConnectionError
#2140
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2140 +/- ##
=======================================
Coverage 92.44% 92.44%
=======================================
Files 104 104
Lines 24385 24393 +8
=======================================
+ Hits 22542 22550 +8
Misses 1843 1843
Continue to review full report at Codecov.
|
6d5c1d4
to
b5d5c30
Compare
@kristjanvalur I tried to push an update to your branch, to fix the linters. Can you run the following to fix your linting? Then we can merge!
|
redis/asyncio/connection.py
Outdated
except OSError as e: | ||
await self.disconnect() | ||
raise ConnectionError( | ||
f"Error while reading from {self.host}:{self.port} : {e.args}" |
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.
Nice. Small nit to remove a space.
f"Error while reading from {self.host}:{self.port}: {e.args}"
redis/connection.py
Outdated
return self._parser.can_read(timeout) | ||
except OSError as e: | ||
self.disconnect() | ||
raise ConnectionError(f"Error while reading from {self.host}:{self.port} : {e.args}") |
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.
Same
f"Error while reading from {self.host}:{self.port}: {e.args}"
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.
Ready to merge pending the lint fix, and this small change!
Pull Request check-list
Please make sure to review and check all of these items:
$ tox
pass with this change (including linting)?NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
This change catches OS errors in
Connection.can_read
, closes connection and raises aredis.ConnectionError
.Fixes #2138