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

Cache condition in helpers.Script #3797

Merged
merged 2 commits into from Oct 11, 2016

Conversation

Projects
None yet
4 participants
@c-w
Copy link
Contributor

commented Oct 10, 2016

Description:

Implementing this as part of Hacktoberfest. The pull-request adds some caching so that we don't always re-create the conditions in the scripts.

The caching is a simple in-memory, per-instance dictionary.
We use __str__ to format cache keys.

This naive implementation has some disadvantages (e.g., we won't be able to cache two conditions that contain references to distinct-but-equivalent object instances and we don't have any control over the size of the condition cache), but for most simple use-cases the approach should be good enough.

Related issue: fixes #3629

Checklist:

  • Local tests with tox run successfully.
  • Tests have been added to verify that the new code works.
Cache condition in helpers.Script
The caching is a simple in-memory, per-instance dictionary.
We use __str__ to format cache keys.

This naive implementation has some disadvantages (e.g., we won't be able
to cache two conditions that contain references to
distinct-but-equivalent object instances and we don't have any control
over the size of the condition cache), but for most simple use-cases the
approach should be good enough.

Resolves #3629
@mention-bot

This comment has been minimized.

Copy link

commented Oct 10, 2016

@c-w, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabianhjr and @gwendalg to be potential reviewers.

@balloob

This comment has been minimized.

Copy link
Member

commented Oct 11, 2016

This is awesome! Thanks 👍 👯‍♂️ 🐬

@balloob balloob merged commit 711526e into home-assistant:dev Oct 11, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.004%) to 93.761%
Details

@home-assistant home-assistant locked and limited conversation to collaborators Mar 17, 2017

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.