Skip to content

Avoid selecting attributes in the history api when no_attributes is passed#68352

Merged
bdraco merged 8 commits intohome-assistant:devfrom
bdraco:no_attr_if_not_needed
Mar 20, 2022
Merged

Avoid selecting attributes in the history api when no_attributes is passed#68352
bdraco merged 8 commits intohome-assistant:devfrom
bdraco:no_attr_if_not_needed

Conversation

@bdraco
Copy link
Copy Markdown
Member

@bdraco bdraco commented Mar 19, 2022

Proposed change

  • We want to get rid of direct database access in integrations, but there currently
    is no way to avoid selecting attributes.

  • Additionally, attribute selection in history database queries make up most of the payload size.
    With minimal_responses we throw away 85%+ of the payload because most history api
    requests do not need attributes and throw them away except for the unit of measurement

  • We can avoid selecting them in the first place for domains that
    do not need them and already have the unit of measurement in memory via
    the state machine

  • Now that attributes are stored in another table we can avoid traversing and
    fetching the data from the database when they aren't needed

  • A new flag no_attributes is added to the history api which can be applied
    when calls already have the attributes needed (currently unit of measurement)

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.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

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

To help with the load of incoming pull requests:

- Most history api requests do not need attributes and throw them
  away. We can avoid selecting them in the first place if we do
  not need them
@probot-home-assistant
Copy link
Copy Markdown

Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration (recorder) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@probot-home-assistant
Copy link
Copy Markdown

Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration (history) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@bdraco bdraco changed the title Avoid selecting attributes when not needed is requested Avoid selecting attributes in the history api when no_attributes is passed Mar 19, 2022
@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Mar 19, 2022

history_stats doesn't need attributes either

So these two history apis need a way to not fetch them as well

        history_list = history.state_changes_during_period(
            self.hass, start, end, str(self._entity_id)
        )
last_state = history.get_state(self.hass, start, self._entity_id)

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Mar 19, 2022

The plant integration does not need attributes either

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Mar 19, 2022

The statistics sensor does need attributes since it looks at the UOM

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Mar 19, 2022

The filter sensor does care about attributes

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Mar 20, 2022

Dropping the attributes on the frontend in testing was

with - Run Time: real 1.891 user 0.041000 sys 0.053000
without - Run Time: real 0.074 user 0.034000 sys 0.040000

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Mar 20, 2022

  • history_states -> no_attr_if_not_needed_history_stats
  • plant -> no_attr_if_not_needed_plant
  • statistics -> no_attr_if_not_needed_statistics

@bdraco bdraco marked this pull request as ready for review March 20, 2022 09:47
@bdraco bdraco requested a review from a team as a code owner March 20, 2022 09:47
@bdraco bdraco merged commit 816695c into home-assistant:dev Mar 20, 2022
@bdraco bdraco deleted the no_attr_if_not_needed branch March 20, 2022 09:47
@github-actions github-actions bot locked and limited conversation to collaborators Mar 21, 2022
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.

3 participants