FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Avoid retrying open_connection on unrecoverable errors by bdraco · Pull Request #340 · python-kasa/python-kasa · GitHub

  1. python-kasa
  2. python-kasa

Repository navigation

Avoid retrying open_connection on unrecoverable errors - #340

Merged
rytilahti merged 2 commits into
python-kasa:masterfrom
bdraco:no_retry_unrecoverable_error
Apr 24, 2022
ConversationCommits (2)ChecksFiles changed
Merged

Avoid retrying open_connection on unrecoverable errors#340
rytilahti merged 2 commits into
python-kasa:masterfrom
bdraco:no_retry_unrecoverable_error

Conversation

bdraco commented Apr 17, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

  • 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

2022-04-18 16:02:03 WARNING (MainThread) [asyncio] Executing <Task finished name='Task-48613' 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.6', 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.002 seconds
2022-04-18 16:02:04 WARNING (MainThread) [asyncio] Executing <Task finished name='Task-48615' 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.6', 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.002 seconds

codecov-commenter commented Apr 17, 2022 •
edited by codecov Bot
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.69%. Comparing base (d2581bf) to head (351fb49).
⚠️ Report is 800 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

bdraco force-pushed the no_retry_unrecoverable_error branch from dc05cc8 to cd398cd Compare April 17, 2022 08:36
Avoid retrying open_connection on unrecoverable errors
20bb804 
- 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
```
bdraco force-pushed the no_retry_unrecoverable_error branch from cd398cd to 20bb804 Compare April 17, 2022 08:40
rytilahti reviewed Apr 23, 2022
View reviewed changes

rytilahti left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

bdraco commented Apr 23, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

I'm fine with this, but is this a real issue considering we only retry a couple of times?

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.

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.

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.

rytilahti commented Apr 24, 2022

Copy link
Copy Markdown
Member

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.

bdraco commented Apr 24, 2022

Copy link
Copy Markdown
Member Author

comment added

rytilahti merged commit d908a5a into python-kasa:master Apr 24, 2022

bdraco commented Apr 24, 2022

Copy link
Copy Markdown
Member Author

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

rytilahti commented Apr 24, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

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.

bdraco commented Apr 24, 2022

Copy link
Copy Markdown
Member Author

I'll have time to fix issues during beta if they come up. Worst case we revert before we publish the stable release

rytilahti added the bug Something isn't working label Apr 24, 2022
rytilahti mentioned this pull request Apr 24, 2022
Merged
bdraco mentioned this pull request May 5, 2022
Closed
pillarsdotnet pushed a commit to pillarsdotnet/python-kasa that referenced this pull request Oct 22, 2026
Avoid retrying open_connection on unrecoverable errors (python-kasa#340)
d1e6912 
* Avoid retrying open_connection on unrecoverable errors

- 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
```

* comment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

rytilahti rytilahti left review comments

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL