Skip to content
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

Make Throttle async aware #13027

Merged
merged 2 commits into from Mar 10, 2018

Conversation

Projects
None yet
4 participants
@balloob
Copy link
Member

commented Mar 9, 2018

Description:

Throttle was not async aware. With coroutines these errors got swallowed but with async/await they surfaced, which is good as now we have been able to squash a bunch of bugs.

Found all instances where we combined an @async.coroutine with a @Throttle. I've converted them all to async/await and also fixed the Throttle decorator.

Found all instances with:

git grep -A3 -B3 Throttle | grep coroutine

Related issue (if applicable): fixes #13012

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@balloob balloob requested a review from home-assistant/core as a code owner Mar 9, 2018

"""Get the latest data from luftdaten.info and update the state."""
try:
yield from self.luftdaten.async_update()
except TypeError:

This comment has been minimized.

Copy link
@balloob

balloob Mar 9, 2018

Author Member

Removed this type error because that would happen if Throttle was returning None

@balloob balloob added this to the 0.65.1 milestone Mar 9, 2018

@balloob balloob merged commit 36361d6 into dev Mar 10, 2018

6 checks passed

WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.01%) to 93.066%
Details
hound No violations found. Woof!

@balloob balloob deleted the throttle-async branch Mar 10, 2018

balloob added a commit that referenced this pull request Mar 10, 2018

Make Throttle async aware (#13027)
* Make Throttle async aware

* Lint

@balloob balloob referenced this pull request Mar 10, 2018

Merged

0.65.1 #13033

@DavidFW1960

This comment has been minimized.

Copy link

commented Mar 10, 2018

Heres the sabnzdb log (I'm home now and I can paste it - not on iPad now)

Sat Mar 10 2018 14:27:14 GMT+1100 (Local Daylight Time)

Error while setting up platform sabnzbd
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/homeassistant/helpers/entity_platform.py", line 82, in async_setup
    SLOW_SETUP_MAX_WAIT, loop=hass.loop)
  File "/usr/lib/python3.6/asyncio/tasks.py", line 358, in wait_for
    return fut.result()
  File "/usr/lib/python3.6/asyncio/futures.py", line 245, in result
    raise self._exception
  File "/usr/lib/python3.6/asyncio/tasks.py", line 180, in _step
    result = coro.send(None)
  File "/usr/lib/python3.6/site-packages/homeassistant/components/sensor/sabnzbd.py", line 153, in async_setup_platform
    if not (yield from async_check_sabnzbd(SabnzbdApi, base_url, api_key)):
  File "/usr/lib/python3.6/site-packages/homeassistant/components/sensor/sabnzbd.py", line 62, in async_check_sabnzbd
    yield from sab_api.check_available()
  File "/usr/lib/python3.6/site-packages/pysabnzbd/__init__.py", line 43, in check_available
    with aiohttp.ClientSession() as session:
  File "/usr/lib/python3.6/site-packages/aiohttp/client.py", line 742, in __enter__
    raise TypeError("Use async with instead")
TypeError: Use async with instead
@balloob

This comment has been minimized.

Copy link
Member Author

commented Mar 10, 2018

sabnzdb has been fixed in #13042

@thomasvt1

This comment has been minimized.

Copy link

commented Mar 11, 2018

I think the Tado component suffers from the same issue (Also like #13012)

2018-03-11 00:56:28 ERROR (MainThread) [homeassistant.core] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/asyncio/tasks.py", line 180, in _step
    result = coro.send(None)
  File "/usr/local/lib/python3.6/site-packages/homeassistant/components/device_tracker/__init__.py", line 738, in async_device_tracker_scan
    found_devices = yield from scanner.async_scan_devices()
  File "/usr/local/lib/python3.6/site-packages/homeassistant/components/device_tracker/tado.py", line 81, in async_scan_devices
    yield from info
  File "/usr/local/lib/python3.6/site-packages/homeassistant/components/device_tracker/tado.py", line 121, in _update_info
    tado_json = yield from response.json()
TypeError: cannot 'yield from' a coroutine object in a non-coroutine generator
@balloob

This comment has been minimized.

Copy link
Member Author

commented Mar 11, 2018

@thomasvt1 thanks for the report, opened #13078 with a fix

hmn added a commit to hmn/home-assistant-dev that referenced this pull request Mar 11, 2018

Merge branch 'dev' into ps4
* dev: (98 commits)
  Sensor template don't exit early on TemplateError (home-assistant#13041)
  - Bump iGlo Version (home-assistant#13063)
  Fixes KNX fire event problem, issue home-assistant#13049 (home-assistant#13062)
  Convert decimals from SQL results
  Convert decimals from SQL results (home-assistant#13059)
  Update frontend to 20180310.0
  Don't call async from sync (home-assistant#13057)
  Ensure we have valid config AFTER merging packages home-assistant#13015 (home-assistant#13038)
  Script/lint, Lazytox: Fix issue to ignore delete files (home-assistant#13051)
  Glances Docker Sensors (home-assistant#13026)
  python-miio version bumped. (home-assistant#13055)
  Yeelight version bumped. (home-assistant#13056)
  Fix async lifx_set_state (home-assistant#13045)
  Fix sensibo's min/max_temp properties (home-assistant#12996)
  Use request.query (home-assistant#13037)
  Bump pysabnzbd version (home-assistant#13042)
  Make lazytox script executable (home-assistant#13040)
  HomeKit Bugfix: names (home-assistant#13031)
  allow ios device tracker see calls to go through (home-assistant#13020)
  Make Throttle async aware (home-assistant#13027)
  ...

@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.