blindpandas / bookworm

The Universally Accessible document Reader
https://getbookworm.com
Other
122 stars 38 forks source link

feat: Updated SQLAlcjhemy to 2.x and introduced alembic for automatic migrations #273

Closed pauliyobo closed 2 days ago

pauliyobo commented 1 week ago

This PR basically allows us to no longer have to keep track of schema versions, since by having alembic running migrations automatically, that'd be done for us. I'm using this as a base in order to implement DB related changes that may solve issues such as #229, #205, and potentially #210 It started as a PR which would target only the integration with alembic, but I had to also upgrade certain PyInstaller tasks, given that the final executable layout has changed ever since v6.x was released. @mush42 @cary-rowen any potential things I might be missing? I'd like to keep the context of this PR as small as possible, given that I've already made it quite broad already

cary-rowen commented 1 week ago

Great, although I haven't looked carefully yet, I think, we can fix the build error first

pauliyobo commented 1 week ago

The build error seems to be fixed. The extra check seems to be related to appveyor, which we no longer rely on.

cary-rowen commented 1 week ago

No, I see the appveyor build is not successful, you can check the log.

The artifacts generated by Github Action are only 200+kb: https://github.com/blindpandas/bookworm/actions/runs/10986077248

cary-rowen commented 1 week ago

Hi @pauliyobo Could you fix the build error?

pauliyobo commented 4 days ago

Hello @cary-rowen feel free to reattempt whenever.

cary-rowen commented 4 days ago

Hi @pauliyobo Below is the error I am getting:

30/09/2024 11:16:23 root CRITICAL: Failed to start Bookworm
30/09/2024 11:16:23 root CRITICAL: A fatal error has occured. Please check the log for more details.
The log has been written to the file:
C:\Users\cary\bookworm.errors.log
30/09/2024 11:16:23 root ERROR: ERROR DETAILS:
Traceback (most recent call last):
  File "sqlalchemy\engine\base.py", line 1967, in _exec_single_context
  File "sqlalchemy\engine\default.py", line 941, in do_execute
sqlite3.OperationalError: table book already exists

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "bookworm\bookworm.py", line 52, in main
  File "bookworm\bootstrap.py", line 230, in run
  File "bookworm\bootstrap.py", line 220, in run
  File "bookworm\bootstrap.py", line 200, in init_app_and_run_main_loop
  File "bookworm\bootstrap.py", line 158, in setupSubsystems
  File "bookworm\database\__init__.py", line 47, in init_database
  File "alembic\command.py", line 406, in upgrade
  File "alembic\script\base.py", line 582, in run_env
  File "alembic\util\pyfiles.py", line 95, in load_python_file
  File "alembic\util\pyfiles.py", line 113, in load_module_py
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "C:\Program Files\Bookworm\_internal\alembic\env.py", line 91, in <module>
    run_migrations_online()
  File "C:\Program Files\Bookworm\_internal\alembic\env.py", line 85, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "alembic\runtime\environment.py", line 946, in run_migrations
  File "alembic\runtime\migration.py", line 628, in run_migrations
  File "C:\Program Files\Bookworm\_internal\alembic\versions\28099038d8d6_initial_migration.py", line 23, in upgrade
    op.create_table('book',
  File "<string>", line 8, in create_table
  File "<string>", line 3, in create_table
  File "alembic\operations\ops.py", line 1311, in create_table
  File "alembic\operations\base.py", line 442, in invoke
  File "alembic\operations\toimpl.py", line 131, in create_table
  File "alembic\ddl\impl.py", line 369, in create_table
  File "alembic\ddl\impl.py", line 210, in _exec
  File "sqlalchemy\engine\base.py", line 1418, in execute
  File "sqlalchemy\sql\ddl.py", line 180, in _execute_on_connection
  File "sqlalchemy\engine\base.py", line 1529, in _execute_ddl
  File "sqlalchemy\engine\base.py", line 1846, in _execute_context
  File "sqlalchemy\engine\base.py", line 1986, in _exec_single_context
  File "sqlalchemy\engine\base.py", line 2355, in _handle_dbapi_exception
  File "sqlalchemy\engine\base.py", line 1967, in _exec_single_context
  File "sqlalchemy\engine\default.py", line 941, in do_execute
sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) table book already exists
[SQL: 
CREATE TABLE book (
    id INTEGER NOT NULL, 
    title VARCHAR(512) NOT NULL, 
    uri VARCHAR(1024) NOT NULL, 
    PRIMARY KEY (id)
)

]
(Background on this error at: https://sqlalche.me/e/20/e3q8)
pauliyobo commented 3 days ago

Hello @cary-rowen Could you try again? We should be taking into account that situation as well now.

cary-rowen commented 3 days ago

Hi @pauliyobo Looks like this fixes the error I mentioned. I'll go ahead and test it out.

cary-rowen commented 3 days ago

Hi @pauliyobo If this PR is merged, will you consider fixing database-related #205, #210 and #229 in the near future?

thank you for your work

pauliyobo commented 2 days ago

Hello. It can certainly be investigated more appropriately now that we don't have to worry about handling migrations. @cary-rowen Does the PR look good to you or have you encountered issues? Or anything I have not addressed?

cary-rowen commented 2 days ago

LGTM, I think we can continue.