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

Restrict recorder query to include max age #18231

Merged
merged 4 commits into from Nov 8, 2018

Conversation

Projects
None yet
5 participants
@ehendrix23
Contributor

ehendrix23 commented Nov 5, 2018

Description:

Added MaxAge to query of recorder if MaxAge is provided to reduce the number of records retrieved from recorder to what is only required.

Checklist:

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

This comment has been minimized.

Member

balloob commented Nov 6, 2018

Requires a test.

@ehendrix23 ehendrix23 force-pushed the ehendrix23:Restrict-recorder-query-to-include-MaxAge branch from 5cebb0c to 5240e4b Nov 6, 2018

@ehendrix23 ehendrix23 changed the title from WIP: Restrict recorder query to include max age to Restrict recorder query to include max age Nov 6, 2018

with patch('homeassistant.components.sensor.statistics.dt_util.utcnow',
new=mock_now), \
patch.object(StatisticsSensor, '_purge_old', mock_purge):

This comment has been minimized.

@houndci-bot

houndci-bot Nov 6, 2018

continuation line over-indented for visual indent

ehendrix23 added some commits Nov 5, 2018

Update query to include maxAge
Updated the query from recorded to include MaxAge if set; reducing the amount of records retrieved that would otherwise be purged anyway for the sensor.
Added newline in docstring
Added newline in docstring.
Add test + small fix
Added test to ensure query works correctly
Query should be greater then or equal instead of greater then. Fixed.
Fixed lint issue
Fixed lint issue.

@ehendrix23 ehendrix23 force-pushed the ehendrix23:Restrict-recorder-query-to-include-MaxAge branch from b2fa750 to a9f1a5f Nov 6, 2018

@dgomes dgomes merged commit ae85baf into home-assistant:dev Nov 8, 2018

5 checks passed

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

@wafflebot wafflebot bot removed the in progress label Nov 8, 2018

@dgomes

This comment has been minimized.

Member

dgomes commented Nov 8, 2018

Quite useful!

Thanks! 🎉

zxdavb added a commit to zxdavb/home-assistant that referenced this pull request Nov 13, 2018

Restrict recorder query to include max age (home-assistant#18231)
* Update query to include maxAge

Updated the query from recorded to include MaxAge if set; reducing the amount of records retrieved that would otherwise be purged anyway for the sensor.

* Added newline in docstring

Added newline in docstring.

* Add test + small fix

Added test to ensure query works correctly
Query should be greater then or equal instead of greater then. Fixed.

* Fixed lint issue

Fixed lint issue.

@ehendrix23 ehendrix23 deleted the ehendrix23:Restrict-recorder-query-to-include-MaxAge branch Nov 16, 2018

@balloob balloob referenced this pull request Nov 29, 2018

Merged

0.83 #18776

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment