Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Oct 1, 2024

Conversation

pauliyobo
Copy link
Collaborator

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
Copy link
Collaborator

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

@pauliyobo
Copy link
Collaborator Author

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

@cary-rowen
Copy link
Collaborator

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
Copy link
Collaborator

Hi @pauliyobo
Could you fix the build error?

@pauliyobo
Copy link
Collaborator Author

Hello
@cary-rowen feel free to reattempt whenever.

@cary-rowen
Copy link
Collaborator

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
Copy link
Collaborator Author

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

@cary-rowen
Copy link
Collaborator

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

@cary-rowen
Copy link
Collaborator

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
Copy link
Collaborator Author

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?

@pauliyobo pauliyobo marked this pull request as ready for review October 1, 2024 15:48
@cary-rowen
Copy link
Collaborator

LGTM, I think we can continue.

@pauliyobo pauliyobo merged commit e050691 into develop Oct 1, 2024
5 of 6 checks passed
@pauliyobo pauliyobo deleted the alembic branch October 2, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants