Skip to content

Improve Plex debounce/throttle logic#33805

Merged
MartinHjelmare merged 6 commits intohome-assistant:devfrom
jjlawren:plex_improve_debouncer
Apr 9, 2020
Merged

Improve Plex debounce/throttle logic#33805
MartinHjelmare merged 6 commits intohome-assistant:devfrom
jjlawren:plex_improve_debouncer

Conversation

@jjlawren
Copy link
Copy Markdown
Contributor

@jjlawren jjlawren commented Apr 7, 2020

Proposed change

I wasn't happy with the update debouncer implementation in #33560 as it would always add at least a 1s delay, and sometimes longer during very busy activity.

New logic:

  • Allow the first update after no activity to fire immediately
  • Throttle frequent updates to 1s intervals
  • Ensure the "last" update is not lost

EDIT: This functionality was already available in homeassistant.helpers.debounce, so I ripped out the custom implementation and used the built-in class instead. (Thanks @balloob!)

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

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

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@jjlawren
Copy link
Copy Markdown
Contributor Author

jjlawren commented Apr 8, 2020

Ended up ripping out the custom debouncer implementation and using the one from homeassistant.helpers.debounce. Most of the diff here is moving certain tests inside of asynctest.ClockedTestCase subclasses to have better control over time-based functionality.

Comment thread tests/components/plex/test_init.py Outdated
Comment thread tests/components/plex/test_init.py
Comment thread homeassistant/components/plex/server.py
@jjlawren jjlawren force-pushed the plex_improve_debouncer branch from a38e619 to 9776c78 Compare April 8, 2020 18:17
Comment thread tests/components/plex/test_server.py Outdated
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Nice!

@MartinHjelmare MartinHjelmare merged commit 42ca566 into home-assistant:dev Apr 9, 2020
@jjlawren jjlawren added this to the 0.108.2 milestone Apr 9, 2020
balloob pushed a commit that referenced this pull request Apr 10, 2020
* Improve Plex debounce/throttle logic

* Use Debouncer helper, rewrite affected tests

* Mock storage so files aren't left behind

* Don't bother with wrapper method, store debouncer call during init

* Test cleanup from review

* Don't patch own code in tests
@balloob balloob mentioned this pull request Apr 10, 2020
@jjlawren jjlawren deleted the plex_improve_debouncer branch April 12, 2020 15:43
@lock lock Bot locked and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants