Migration from older profile version

closes #403

added Review Topic::CLI Topic::TUI labels

assigned to @msobczyk

added 12 commits

• 75ed4d45...7b992ff8 - 2 earlier commits
• 03a71155 - Get rid of clive.__private.core.date_utils dependency in storage
• 269c7aed - Include alarms model in calculating storage revision
• 82ebe211 - Use transaction from schemas as model to store transaction in profile
• f11c278c - Reimport and use new models instead of old storage models
• 1769c219 - Detect multiple storage versions in PersistentStorageService
• ce0aa581 - Fix unit tests after storage revision change
• 450342a2 - New unit tests for detecting multiple storage versions
• f1b64287 - Introduce enum for storage version
• 14cb090d - New cli tests with loading old profile and migration
• bacab5f9 - Additional unit tests for profile with alarms or operations stored

Compare with previous version

added 15 commits

• bacab5f9...885fbabc - 5 earlier commits
• 8f470313 - Reimport and use new models instead of old storage models
• 8e872d3a - Show also storage model version number in cli command clive --version
• 01b9d8e8 - Detect multiple storage versions in PersistentStorageService
• 5ed5a650 - Add --force flag to configure profile remove required to delete multiple storage versions
• c6da9503 - Fix unit test for storage revision change
• 8775f931 - New unit tests for migration, listing and deleting multiple storage versions
• c1001a81 - New cli tests for handling older storage versions
• 9fb4de51 - Introduce new storage version with additional tui_theme field in profile
• 8ced1542 - Fix unit tests for storage revision
• 08eac5a9 - Save and load tui theme from storage in profile

Compare with previous version

added 2 commits

• 4593cbdf - Fix unit tests for storage revision
• d30b3136 - Save and load tui theme from storage in profile

Compare with previous version

marked this merge request as ready

Looks like this command should have name="delete"

I think saving theme doesn't work correctly when it's not changed for the first time after unlock:

• change theme on unlock to eg textual-light
• unlock (the same theme textual-light will be set)
• lock
• change theme to e.g gruvbox
• unlock (the same theme gruvbox will be set)

This means if someone won't set theme after unlock - he can see different theme if someone else changes it on the Unlock screen

Looks like there is a missing _update_theme_in_profile after unlock?

I wonder if it will be better to have this only as str and the default should be DEFAULT_THEME? like from textual.constants or we should have our own CLIVE_DEFAULT_THEME defined in settings.toml

Why isn't it a part of app._switch_mode_into_unlocked ?

Better to provide theme: str param and pass it instead of self.theme. This prevents race conditions (which may not occur here but it's a good manner)

I think it would be much easier to maintain the storage if we'll have something simialr to https://docs.pydantic.dev/latest/api/json_schema/#pydantic.json_schema.SkipJsonSchema instead of defining new class structure for calculating the storage revision

Maybe it would be cleaner to define additional schemas under main namespace like: (But wait with that for later, we'll see what it looks like if other threads are resolved)

from __future__ import annotations

from pathlib import Path
from typing import Any

from pydantic import BaseModel


class ProfileStorageModel(BaseModel):
    class TransactionStorageModel(BaseModel):
        transaction: Any
        transaction_file_path: Path

    name: str
    transaction: ProfileStorageModel.TransactionStorageModel


class NewerProfileStorageModel(ProfileStorageModel):
    name: int


class NewestProfileStorageModel(NewerProfileStorageModel):
    class TransactionStorageModel(NewerProfileStorageModel.TransactionStorageModel):
        additional: str

    name: str
    transaction: NewestProfileStorageModel.TransactionStorageModel | None = None


print(ProfileStorageModel.schema_json(indent=4))
print("newer:")
print(NewerProfileStorageModel.schema_json(indent=4))
print("newest:")
print(NewestProfileStorageModel.schema_json(indent=4))

These two can be defined only once - as classmethods of ProfileStorageModel

maybe use StrEnum and auto() ?

This can be also defined under Version namespace

Why do we need this Enum at all and additional V1, V2 etc while we have revision hash? To compare revisions? Looks like this can be done e.g. based on the list index like greater the index - the newer the revision

This does not include v3 but works fine and mypy is not complying. Maybe no need for that? Less places to edit while adding a new version. How many places now should be edited to add a new version? Maybe can you provide some instruction in docstring?

So every migration (data conversion) will be placed there? This will become very overcomplicated pretty soon.. Maybe we should have methods like def upgrade defined at the ProfileStorageModel level? or something like upgrade_v1_to_v2 but not everything stored in a single function.