-
-
Notifications
You must be signed in to change notification settings - Fork 32.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
Convert sense to use DataUpdateCoordinator for trends data #34160
Convert sense to use DataUpdateCoordinator for trends data #34160
Conversation
Sense was having api performance issue this afternoon. Its not blocking anything now. So should be good to go. |
|
||
# async_request_refresh instead of async_refresh | ||
# as this can take longer than 60s | ||
await trends_coordinator.async_request_refresh() |
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.
This will still trigger the fetch and block until it is finished.
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.
Oh nevermind, default debouncer of update coordinator is not set to immediate
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.
Uuh, I was wrong. It is set to immediate by default. So this is doing a full refresh.
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.
Thanks. Switched to async_call_later
unless you can suggest something better?
Co-Authored-By: Paulus Schoutsen <paulus@home-assistant.io>
Co-Authored-By: Paulus Schoutsen <paulus@home-assistant.io>
Retested with changes! TEST PASS Thanks @balloob |
* Convert sense to use DataUpdateCoordinator for trends * remove unused * request update right away * clarify * call async refresh later * Update homeassistant/components/sense/__init__.py Co-Authored-By: Paulus Schoutsen <paulus@home-assistant.io> * Update homeassistant/components/sense/__init__.py Co-Authored-By: Paulus Schoutsen <paulus@home-assistant.io> Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>
Proposed change
Sense sometimes fails to startup because update_trend_data takes longer than 60 seconds.
Convert sense to use DataUpdateCoordinator for trends data
Type of change
Example entry for
configuration.yaml
:# Example configuration.yaml
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale: