Skip to content

Commit f252a86

Browse files
refactor: split out sql migrations from library (#1432)
* refactor: minor simplification * refactor: split open_library into new and not new case Functionality is entirely unchanged, but entire method is duplicated. Removing dead code from either copy is next. * refactor: remove dead code * doc: add todos and remove trivially true assert * refactor: add assurance 1 version check Assurance 1 was introduced with library version 101, specifically commmit 12e074b. Thus all libraries created on version 101 and above already fullfill it and don't need it. Furthermore, it only fails if the library didn't need the assurance, meaning the try-except and warning catching can be removed * refactor: remove various unnecessary try-except statements in new_lib All of these were introduced to account for the differences between new libraries and existing ones, which makes them unnecessary after this split. * refactor: remove unnecessary check in new_lib * refactor: move folder assurance after migrations The folder assurance has been present since the very first SQL commit e5e7b8a, and thus only has an effect when the library is moved, meaning that is semantically not part of the migrations. * refactor: massively simplify open_library * refactor: move engine creation to static method * refactor: add version check for assurance 3 Assurance 3 was introduced in commit 47c3d53 shortly (24h 20min) after the DB version had been bumped to 200. Since it was also included in the first release with DB version 200, all libraries created on DB version 200 and above can be presumed to have already had this applied on creation. * refactor: add assurance 3 to DB 200 migration * refactor: move assurance 1 to a proper migration method Moving the assurance after the migrations 7, 8, 9, and 100 is fine because it doesn't affect those. * refactor: update version after every successfull migration Since every migration migrates the library to a certain DB_VERSION, we can simply update the library version after such a successfull migration. This way if a migration fails, the previous migrations won't be rerun the next time the library is opened. * refactor: rewrite migration procedure as loop * refactor: apply migration and update version in same transaction None of the content of the migrations is changed, only the `with session:` statements are removed. * refactor: replace all commits in the migrations with flushes Also removes various try-except statements in the migrations. These were introduced to catch the case where the migration is run twice due to a later migration failing, this is not necessary anymore as every migration will only be executed at most once per library. * refactor: make sure the migration log statements are consistent * fix: pass library dir to migrations * fix(db migration 8): only add colors that are actually new DB Migration 8 was previously adding all colors except for the neon ones, however, all but three of those have been around since DB version 4, meaning that they don't need to be added at all (which caused the tests to fail). * fix: json migration used outdated interface * fix(open_library): create TS directory only if not opened in memory * fix: enable sane transaction behaviour By default in SQLAlchemy schema changes are automatically committed, even if auto-commit is turned off. This commit turns off auto-commit completely, which caused some schema to not be applied correctly, but those were fixed here as well. * refactor: hide 'argument is not accessed' notices * fix(db migration 9): filename property wasn't written correctly * fix(db migration 104): include/exclude list was loaded incorrectly * feat: log start and end of DB migrations * fix: don't use double transaction in open_sqlite_library * fix: only commit once when creating new library * doc: add comment on removing Folder logic * refactor: remove dunder naming
1 parent a0fb679 commit f252a86

3 files changed

Lines changed: 455 additions & 474 deletions

File tree

src/tagstudio/core/library/alchemy/db.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,17 @@ def make_engine(connection_string: str) -> Engine:
4242

4343
def make_tables(engine: Engine) -> None:
4444
logger.info("[Library] Creating DB tables...")
45-
Base.metadata.create_all(engine)
46-
47-
# tag IDs < 1000 are reserved
48-
# create tag and delete it to bump the autoincrement sequence
49-
# TODO - find a better way
50-
# is this the better way?
5145
with engine.connect() as conn:
46+
# TODO: this should instead be migrations that create the exact tables that were added in
47+
# the respective DB versions
48+
Base.metadata.create_all(conn)
49+
conn.commit()
50+
51+
# TODO: this needs to be a migration
52+
# tag IDs < 1000 are reserved
53+
# create tag and delete it to bump the autoincrement sequence
54+
# TODO - find a better way
55+
# is this the better way?
5256
result = conn.execute(text("SELECT SEQ FROM sqlite_sequence WHERE name='tags'"))
5357
autoincrement_val = result.scalar()
5458
if not autoincrement_val or autoincrement_val <= RESERVED_TAG_END:

0 commit comments

Comments
 (0)