Avoid retrying open_connection on unrecoverable errors#340
Avoid retrying open_connection on unrecoverable errors#340rytilahti merged 2 commits intopython-kasa:masterpython-kasa/python-kasa:masterfrom bdraco:no_retry_unrecoverable_errorbdraco/python-kasa:no_retry_unrecoverable_errorCopy head branch name to clipboard
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #340 +/- ##
==========================================
+ Coverage 80.52% 80.69% +0.17%
==========================================
Files 25 25
Lines 1715 1725 +10
Branches 237 240 +3
==========================================
+ Hits 1381 1392 +11
+ Misses 300 299 -1
Partials 34 34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dc05cc8 to
cd398cd
Compare
- We can retry so hard that we block the event loop Fixes ``` 2022-04-16 22:18:51 WARNING (MainThread) [asyncio] Executing <Task finished name=Task-3576 coro=<open_connection() done, defined at /opt/homebrew/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/streams.py:25> exception=ConnectionRefusedError(61, "Connect call failed (192.168.107.200, 9999)") created at /opt/homebrew/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/tasks.py:460> took 1.001 seconds ```
cd398cd to
20bb804
Compare
rytilahti
left a comment
There was a problem hiding this comment.
I'm fine with this, but is this a real issue considering we only retry a couple of times?
Also, did you try to add a short sleep between calls to see if that'd be enough? Having a pause in-between might (or also might not) also alleviate issues where the device is not responsive for the initial request and has no time to wake up to answer to the subsequent ones.
When I lost an access point due to a POE injector getting fried, my instance became unresponsive due to the event loop being blocked from all the retries.
In general the connection should already be open so we shouldn't have to retry so aggressively now. I would expect to get a timeout, and not one of the errors being checked here if the device is not responding right away which is still being handled the same. |
True, please add a short comment about this special handling for future reference and then it is good to go. |
|
comment added |
|
Would you do a new release? I think it would be a good time to bump home assistant before beta so we can address anything with modularize that needs tweaks |
|
I was a bit hesitant to prepare a new one as the modularize merge touched so many internals and as I won't have much time to handle potential issues, but I will prepare 0.5.0 and if needed homeassistant can always step back using the current release. |
|
I'll have time to fix issues during beta if they come up. Worst case we revert before we publish the stable release |
This one should probably get a new release
We can retry so hard that we block the event loop
Fixes
2022-04-16 22:18:51 WARNING (MainThread) [asyncio] Executing <Task finished name=Task-3576 coro=<open_connection() done, defined at /opt/homebrew/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/streams.py:25> exception=ConnectionRefusedError(61, "Connect call failed (192.168.107.200, 9999)") created at /opt/homebrew/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/tasks.py:460> took 1.001 seconds