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 it possible to inherit EntityDescription in frozen and mutable dataclasses #105211

Merged
merged 6 commits into from
Dec 11, 2023

Conversation

emontnemery
Copy link
Contributor

@emontnemery emontnemery commented Dec 7, 2023

Proposed change

Make it possible to inherit EntityDescription in both frozen and mutable dataclasses

Background and rationale

PR #100601 explores caching of entity properties. A major issue is that although not useful, entity descriptions are mutable, meaning we may miss changes if an integration mutates its entity descriptions.
If we make EntityDescription a frozen dataclass the problem is solved for all core integrations, but this would be a hard breaking change for custom integrations because it's a runtime error to derive a mutable dataclass from a frozen dataclass.

More details here: #100601 (comment)

Solution

Make EntityDescription behave as a dataclass, but allow it to be the base class for both frozen and mutable dataclasses. This is achieved with a metaclass FrozenOrThawed. Thanks to PEP 681 type checkers, including MyPy, keep working.

The FrozenOrThawed metaclass will look for the class argument frozen_or_thawed:

  • If the flag is set on a class, the class will not be a dataclass but will behave as a dataclass.
  • If the flag is not set, the metaclass does nothing and assumes the class is decorated with @dataclass.

In follow-up PRs:

  • Entity descriptions of base components will also have this flag set.
  • Freeze entity descriptions of all core integrations
  • Log deprecation warnings if dataclasses derived from EntityDescription are not frozen

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)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format 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.

To help with the load of incoming pull requests:

@emontnemery emontnemery requested a review from a team as a code owner December 7, 2023 10:24
@emontnemery emontnemery marked this pull request as draft December 7, 2023 10:24
@emontnemery emontnemery marked this pull request as ready for review December 7, 2023 10:24
@emontnemery emontnemery requested review from a team and bdraco as code owners December 7, 2023 14:12
homeassistant/helpers/entity.py Show resolved Hide resolved
homeassistant/helpers/entity.py Outdated Show resolved Hide resolved
@emontnemery emontnemery closed this Dec 7, 2023
@emontnemery emontnemery reopened this Dec 7, 2023
@bdraco
Copy link
Member

bdraco commented Dec 7, 2023

This looks good

Should we do a blog post about freezing entity description dataclasses so we have a path forward to removing the compat layer?

@emontnemery
Copy link
Contributor Author

This looks good

Should we do a blog post about freezing entity description dataclasses so we have a path forward to removing the compat layer?

Blog post added 👍

@bdraco
Copy link
Member

bdraco commented Dec 11, 2023

Pushing the latest to my production now

@bdraco
Copy link
Member

bdraco commented Dec 11, 2023

All good on production, nothing obvious blew up

@frenck frenck merged commit dd33879 into dev Dec 11, 2023
53 checks passed
@frenck frenck deleted the freeze_entity_description branch December 11, 2023 19:00
Bre77 pushed a commit to Bre77/core that referenced this pull request Dec 12, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 12, 2023
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