get_connection: catch OSError too

[Enchufa2]

which covers built-in ConnectionError, to recover from connection issues such as ConnectionResetError and ConnectionAbortedError. Closes #1772.

Pull Request check-list

• Does $ tox pass with this change (including linting)?
• Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
• Is the new or changed code fully tested?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
• Is there an example added to the examples folder (if applicable)?

[codecov-commenter]

Codecov Report

Merging #1832 (bdbded7) into master (04b8d34) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1832   +/-   ##
=======================================
  Coverage   94.41%   94.41%           
=======================================
  Files          75       75           
  Lines       16026    16026           
=======================================
  Hits        15131    15131           
  Misses        895      895           

Impacted Files	Coverage Δ
redis/connection.py	88.81% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04b8d34...bdbded7. Read the comment docs.

[elemoine]

Why wasn't this change applied to BlockingConnectionPool as well?

[Enchufa2]

Good catch. Maybe that part should be refactored to avoid repeating code, and this kind of problem.

[elemoine]

Good catch. Maybe that part should be refactored to avoid repeating code, and this kind of problem.

Thanks for confirming.

[elemoine]

See #2001 that applies the same change to BlockingConnectionPool.

[hyeongguen-song]

Hi, @Enchufa2
Why asyncio.ConnectionPool don't catch OSError?

redis-py/redis/asyncio/connection.py

Line 1448 in 936d49f

except ConnectionError:

[Enchufa2]

@hyeongguen-song Because asyncio support was added 8 months ago, but this PR was opened 9 months ago. ;-) So, yeap, this should be ported to asyncio too.

[hyeongguen-song]

@hyeongguen-song Because asyncio support was added 8 months ago, but this PR was opened 9 months ago. ;-) So, yeap, this should be ported to asyncio too.

Thanks for reply 😉