Remove recorder PostSchemaMigrationTask #125076
Conversation
home-assistant
bot
added
cla-signed
core
integration: recorder
small-pr
|
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
| # columns to be timestamps. In version 32 we need to wipe the old columns | ||
| # since they are no longer used and take up a significant amount of space. | ||
| assert self.instance.engine is not None, "engine should never be None" | ||
| _wipe_old_string_time_columns(self.instance, self.instance.engine, session) |
There was a problem hiding this comment.
This is limited to 100k rows for MySQL / MariaDB / PostgreSQL, additional rows will eventually be purged. Should we instead loop until there are no more rows?
There was a problem hiding this comment.
I figured it wasn't urgent to delete them and it was better to let them get cleaned up over time so the migration didn't take too long. At this point since most people have been migrated it might make more sense to do the full cleanup instead of waiting for purge.
There was a problem hiding this comment.
OK, that makes sense 👍
|
I think there's a problem with test coverage, because when applying the patch below on top of the PR - which makes the post migrators not run - all recorder tests still pass. diff --git a/homeassistant/components/recorder/migration.py b/homeassistant/components/recorder/migration.py
index 213462e3731..d0263ffbef3 100644
--- a/homeassistant/components/recorder/migration.py
+++ b/homeassistant/components/recorder/migration.py
@@ -1401,6 +1401,7 @@ class _SchemaVersion32Migrator(_SchemaVersionMigrator, target_version=32):
_drop_index(self.session_maker, "events", "ix_events_event_type_time_fired")
_drop_index(self.session_maker, "states", "ix_states_last_updated")
_drop_index(self.session_maker, "events", "ix_events_time_fired")
+ return
with session_scope(session=self.session_maker()) as session:
# In version 31 we migrated all the time_fired, last_updated, and last_changed
# columns to be timestamps. In version 32 we need to wipe the old columns
@@ -1488,6 +1489,7 @@ class _SchemaVersion35Migrator(_SchemaVersionMigrator, target_version=35):
# In version 34 we migrated all the created, start, and last_reset
# columns to be timestamps. In version 35 we need to wipe the old columns
# since they are no longer used and take up a significant amount of space.
+ return
while not cleanup_statistics_timestamp_migration(self.instance):
pass |
Proposed change
Remove recorder
PostSchemaMigrationTaskwhich took care of additional cleanup after migration to schema versions 32 and 35.Migration to schema versions 32 and 35 is done before Home Assistant starts, and by also doing the additional cleanup before Home Assistant starts the state is well defined once the non-live migration tasks are completed.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: