feat: store `Entry` suffix separately #503

yedpodtrzitko · 2024-09-14T05:33:01Z

Initially the main and only goal was to add the column for file suffix into Entry table.

However having a new non-nullable column doesnt work with a previously created DB, so I added a popup showing error to make it obvious why it's not working.

To write tests more easily, the library-opening-evaluating was extracted to standalone method, and eventually even to a standalone class, since this functionality is (should be) independent from QT (well... beside the QSettings).

CyanVoxel

Approved when suggestion is resolved 👍

CyanVoxel · 2024-09-27T21:04:24Z

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

+            # if the db version is different, we cant proceed
+            if db_version.value != LibraryPrefs.DB_VERSION.value:
+                logger.error(
+                    "DB version mismatch",
+                    db_version=db_version.value,


Could there be a log message explaining to the user that there's a library version mismatch? Otherwise it's a bit difficult to glean that from the resulting traceback

that's handled by following code via dialog window, since not every user will be able to see logs:

https://github.com/TagStudioDev/TagStudio/pull/503/files#diff-7a6a5e7532210b0f0683b23e80516e331b21c10efca45cce62ca467690cccb8eR1074-R1077

Interesting, I don't get that on my machine. It just locks up and throws a traceback in the console:

Traceback (most recent call last): File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1967, in _exec_single_context self.dialect.do_execute( File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\default.py", line 941, in do_execute cursor.execute(statement, parameters) sqlite3.OperationalError: no such column: entries.suffix The above exception was the direct cause of the following exception: Traceback (most recent call last): File "F:\GitHub\TagStudio/tagstudio/tag_studio.py", line 65, in main driver.start() File "F:\GitHub\TagStudio/tagstudio\src\qt\ts_qt.py", line 471, in start self.open_library(path_result.library_path) File "F:\GitHub\TagStudio/tagstudio\src\qt\ts_qt.py", line 1094, in open_library self.filter_items() File "F:\GitHub\TagStudio/tagstudio\src\qt\ts_qt.py", line 1000, in filter_items results = self.lib.search_library(self.filter) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "F:\GitHub\TagStudio/tagstudio\src\core\library\alchemy\library.py", line 443, in search_library count_all: int = session.execute(query_count).scalar() ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 2362, in execute return self._execute_internal( ^^^^^^^^^^^^^^^^^^^^^^^ File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 2247, in _execute_internal result: Result[Any] = compile_state_cls.orm_execute_statement( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\context.py", line 293, in orm_execute_statement result = conn.execute( ^^^^^^^^^^^^^ File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1418, in execute return meth( ^^^^^ File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\sql\elements.py", line 515, in _execute_on_connection return connection._execute_clauseelement( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1640, in _execute_clauseelement ret = self._execute_context( ^^^^^^^^^^^^^^^^^^^^^^ File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1846, in _execute_context return self._exec_single_context( ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1986, in _exec_single_context self._handle_dbapi_exception( File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 2355, in _handle_dbapi_exception raise sqlalchemy_exception.with_traceback(exc_info[2]) from e File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1967, in _exec_single_context self.dialect.do_execute( File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\default.py", line 941, in do_execute cursor.execute(statement, parameters) sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) no such column: entries.suffix [SQL: SELECT count(*) AS count_1 FROM (SELECT entries.id AS id, entries.folder_id AS folder_id, entries.path AS path, entries.suffix AS suffix FROM entries WHERE (entries.suffix NOT IN (?, ?, ?))) AS entries] [parameters: ('.json', '.xmp', '.aae')] (Background on this error at: https://sqlalche.me/e/20/e3q8) TagStudio Frontend (Qt) Crashed! Press Enter to Continue... 2024-09-27 16:43:37 [info ] Directory scan time duration=0.2599177360534668 new_files_count=246 path=WindowsPath('F:/Example SQL')

That was interesting bug.

Initially I created LibraryPrefs as Enum. Enums cant have multiple identical values, and this one actually had it without me realizing it:

LibraryPrefs.DB_VERSION was 1

LibraryPrefs.IS_EXCLUDE_LIST was True, which gets also evaluated to 1

Since LibraryPrefs.IS_EXCLUDE_LIST came first, DB_VERSION was totally ignored.

This PR changes the value of DB_VERSION to 2, which is not identical with value of IS_EXCLUDE_LIST anymore, so the property DB_VERSION was actually recognized this time. So when I was testing it with value 2 vs value 3, it was working as expected.

But since it didnt exist in your database at all, it created there value of 2.
As such the DB_VERSION was matching the expectations, so no error was displayed.

I changed the LibraryPrefs from the default Enum to custom implementation of Enum, which can keep multiple identical values.

After creating a fresh library from main then loading it here, the popup seems to work now. Let me know if this is working as expected, with previous libraries being labeled as v0 (no issue with it, just want to make sure we're matching).

I'm also presuming that the test library I was using to test this previously is meant to throw a similar error due to the preexisting values inside it. If not, I've included the traceback just for completeness sake:

Traceback

2024-10-02 13:07:54 [info ] resources registered count=4 2024-10-02 13:07:54 [info ] Config File not specified, using default one filename=C:/Users/Faceboji/AppData/Roaming/TagStudio/TagStudio.ini 2024-10-02 13:08:03 [info ] update_widgets selected=[] 2024-10-02 13:08:03 [info ] opening library connection_string=sqlite:///F:\Example SQL\.TagStudio\ts_library.sqlite library_dir=WindowsPath('F:/Example SQL') 2024-10-02 13:08:03 [info ] creating db tables F:\GitHub\TagStudio/tagstudio\src\core\library\alchemy\library.py:186: SAWarning: New instance with identity key (, ('DB_VERSION',), None) conflicts with persistent instance session.commit() 2024-10-02 13:08:04 [info ] update_widgets selected=[] Traceback (most recent call last): File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1967, in _exec_single_context self.dialect.do_execute( File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\engine\default.py", line 941, in do_execute cursor.execute(statement, parameters) sqlite3.OperationalError: no such column: entries.suffix
The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "F:\GitHub\TagStudio/tagstudio/tag_studio.py", line 65, in main
driver.start()
File "F:\GitHub\TagStudio/tagstudio\src\qt\ts_qt.py", line 470, in start
self.open_library(path_result.library_path)
File "F:\GitHub\TagStudio/tagstudio\src\qt\ts_qt.py", line 1093, in open_library
self.filter_items()
File "F:\GitHub\TagStudio/tagstudio\src\qt\ts_qt.py", line 999, in filter_items
results = self.lib.search_library(self.filter)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "F:\GitHub\TagStudio/tagstudio\src\core\library\alchemy\library.py", line 459, in search_library
count_all: int = session.execute(query_count).scalar()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 2362, in execute
return self._execute_internal(
^^^^^^^^^^^^^^^^^^^^^^^
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 2247, in _execute_internal
result: Result[Any] = compile_state_cls.orm_execute_statement(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\orm\context.py", line 293, in orm_execute_statement
result = conn.execute(
^^^^^^^^^^^^^
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1418, in execute
return meth(
^^^^^
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\sql\elements.py", line 515, in _execute_on_connection
return connection._execute_clauseelement(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1640, in _execute_clauseelement
ret = self._execute_context(
^^^^^^^^^^^^^^^^^^^^^^
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1846, in _execute_context
return self._exec_single_context(
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1986, in _exec_single_context
self._handle_dbapi_exception(
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 2355, in _handle_dbapi_exception
raise sqlalchemy_exception.with_traceback(exc_info[2]) from e
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\engine\base.py", line 1967, in _exec_single_context
self.dialect.do_execute(
File "f:\GitHub\TagStudio.venv\Lib\site-packages\sqlalchemy\engine\default.py", line 941, in do_execute
cursor.execute(statement, parameters)
sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) no such column: entries.suffix
[SQL: SELECT count(*) AS count_1
FROM (SELECT entries.id AS id, entries.folder_id AS folder_id, entries.path AS path, entries.suffix AS suffix
FROM entries
WHERE (entries.suffix NOT IN (?, ?, ?))) AS entries]
[parameters: ('.json', '.xmp', '.aae')]
(Background on this error at: https://sqlalche.me/e/20/e3q8)

TagStudio Frontend (Qt) Crashed! Press Enter to Continue...
2024-10-02 13:08:04 [info ] Directory scan time duration=0.46912550926208496 new_files_count=246 path=WindowsPath('F:/Example SQL')

If all this is working as intended, then I'm good to approve it 👍

After creating a fresh library from main then loading it here, the popup seems to work now. Let me know if this is working as expected, with previous libraries being labeled as v0

yes, this is the intended behaviour

yedpodtrzitko force-pushed the yed/extensions-table branch 13 times, most recently from b99ddfa to c0427d7 Compare September 14, 2024 20:30

CyanVoxel added Type: Refactor Code that needs to be restructured or cleaned up TagStudio: Library Relating to the TagStudio library system Status: Review Needed A review of this is needed labels Sep 14, 2024

yedpodtrzitko force-pushed the yed/extensions-table branch from c0427d7 to b5bb104 Compare September 15, 2024 02:09

CyanVoxel reviewed Sep 27, 2024

View reviewed changes

CyanVoxel removed the Status: Review Needed A review of this is needed label Sep 27, 2024

feat: save entry suffix separately

9f1f460

yedpodtrzitko force-pushed the yed/extensions-table branch 5 times, most recently from ba59730 to a8cb58e Compare September 28, 2024 03:58

change LibraryPrefs to allow identical values, add test

5a067e0

yedpodtrzitko force-pushed the yed/extensions-table branch from a8cb58e to 5a067e0 Compare September 28, 2024 03:58

yedpodtrzitko requested a review from CyanVoxel September 29, 2024 09:13

CyanVoxel approved these changes Oct 3, 2024

View reviewed changes

CyanVoxel added the Status: Mergeable The code is ready to be merged label Oct 3, 2024

CyanVoxel merged commit e075282 into TagStudioDev:main Oct 7, 2024
5 checks passed

CyanVoxel removed the Status: Mergeable The code is ready to be merged label Oct 7, 2024

CyanVoxel mentioned this pull request Oct 7, 2024

[Bug]: Can't Create Libraries #535

Closed

3 tasks

yedpodtrzitko deleted the yed/extensions-table branch November 18, 2024 16:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: store `Entry` suffix separately #503

feat: store `Entry` suffix separately #503

yedpodtrzitko commented Sep 14, 2024

CyanVoxel left a comment

CyanVoxel Sep 27, 2024

yedpodtrzitko Sep 27, 2024

CyanVoxel Sep 27, 2024

yedpodtrzitko Sep 28, 2024

CyanVoxel Oct 2, 2024

yedpodtrzitko Oct 3, 2024

feat: store Entry suffix separately #503

feat: store Entry suffix separately #503

Conversation

yedpodtrzitko commented Sep 14, 2024

CyanVoxel left a comment

Choose a reason for hiding this comment

CyanVoxel Sep 27, 2024

Choose a reason for hiding this comment

yedpodtrzitko Sep 27, 2024

Choose a reason for hiding this comment

CyanVoxel Sep 27, 2024

Choose a reason for hiding this comment

yedpodtrzitko Sep 28, 2024

Choose a reason for hiding this comment

CyanVoxel Oct 2, 2024

Choose a reason for hiding this comment

yedpodtrzitko Oct 3, 2024

Choose a reason for hiding this comment

feat: store `Entry` suffix separately #503

feat: store `Entry` suffix separately #503