Fix flask_migrate setup #2432
Fix flask_migrate setup #2432
Conversation
matrss
force-pushed
the
fix-migrations
branch
6 times, most recently
from
18bdaff
to
78865ca
Compare
matrss
force-pushed
the
fix-migrations
branch
3 times, most recently
from
c1111d0
to
7815368
Compare
|
seen, want to review in the next break |
| instead of adding a new one. | ||
| You can still generate a script with the above command first | ||
| to get a starting point for the changes. | ||
|
|
There was a problem hiding this comment.
ok, that's also an option, then maybe we should have a test that the naming convention is used.
I assume a model change needs now a mig script or our test setup will cry.
There was a problem hiding this comment.
I assume a model change needs now a mig script or our test setup will cry.
Yes, that already happens due to the added tests.
maybe we should have a test that the naming convention is used.
We would first have to define an actual naming convention. Naming the migration might not even be possible until a release is cut, since the next version is not known at the time of creation of the migration script.
| db.session.add(db_perm) | ||
| db.session.commit() | ||
| db.session.close() | ||
| # create users |
There was a problem hiding this comment.
observation:
using seed, reset now downgrades first
(mssdev) reimar@infinity:~/PycharmProjects/2023/matrss/MSS$ python mslib/mscolab/mscolab.py db --seed
WARNING:root:Couldn't import mscolab_settings (ImportError:'No module named 'mscolab_settings''), using dummy config.
WARNING:root:Couldn't import setup_saml2_backend (ImportError:'No module named 'setup_saml2_backend''), using dummy config.
INFO [alembic.runtime.migration] Context impl SQLiteImpl.
INFO [alembic.runtime.migration] Will assume non-transactional DDL.
Database initialised successfully!
WARNI [root] Couldn't import mscolab_auth (ImportError:'{No module named 'mscolab_auth'), creating dummy config.
Are you sure you want to seed the database? Seeding will delete all your existing data and replace it with seed data (y/[n]):y
INFO [alembic.runtime.migration] Context impl SQLiteImpl.
INFO [alembic.runtime.migration] Will assume non-transactional DDL.
INFO [alembic.runtime.migration] Running downgrade 922e4d9c94e2 -> c171019fe3ee, To version 10.0.0
INFO [alembic.runtime.migration] Running downgrade c171019fe3ee -> 92eaba86a92e, To version 9.0.0
INFO [alembic.runtime.migration] Running downgrade 92eaba86a92e -> , To version 8.3.5 - Initial migration
INFO [alembic.runtime.migration] Context impl SQLiteImpl.
INFO [alembic.runtime.migration] Will assume non-transactional DDL.
INFO [alembic.runtime.migration] Running upgrade -> 92eaba86a92e, To version 8.3.5 - Initial migration
INFO [alembic.runtime.migration] Running upgrade 92eaba86a92e -> c171019fe3ee, To version 9.0.0
INFO [alembic.runtime.migration] Running upgrade c171019fe3ee -> 922e4d9c94e2, To version 10.0.0
Database seeded successfully!
another db option doesn't
python mslib/mscolab/mscolab.py db --add_all_to_all_operation
WARNING:root:Couldn't import mscolab_settings (ImportError:'No module named 'mscolab_settings''), using dummy config.
WARNING:root:Couldn't import setup_saml2_backend (ImportError:'No module named 'setup_saml2_backend''), using dummy config.
INFO [alembic.runtime.migration] Context impl SQLiteImpl.
INFO [alembic.runtime.migration] Will assume non-transactional DDL.
Database initialised successfully!
WARNI [root] Couldn't import mscolab_auth (ImportError:'{No module named 'mscolab_auth'), creating dummy config.
Are you sure you want to add users to the ALL operations? (y/[n]):y
Maybe also seed and reset can stay on the recent version?
There was a problem hiding this comment.
What it does now is a downgrade to the base revision (i.e. a empty database) and then upgrade again. This drops all tables and recreates them, removing all content along the way.
This is the same operation that reset already did (drop_all followed by create_all), just rephrased in terms of flask_migrate. I don't really see a reason to change that, unless it ever becomes unbearably slow due to too many migrations.
Doing it this way is even a must, if we understand reset not just in terms of the data in the database, but also in terms of the schema of the database. Imagine someone (accidentally?) modifies the schema, e.g. adds a column or an index, to the database. Now, if reset didn't do a downgrade + upgrade but just deleted all rows it wouldn't be able to reset the schema back to what it should be.
| sys.exit( | ||
| """Your database contains no alembic_version revision identifier, but it has a schema. This suggests \ | ||
| that you have a pre-existing database but haven't followed the database migration instructions. To prevent damage to \ | ||
| your database MSColab will abort. Please follow the documentation for a manual database migration from MSColab v8/v9.""" | ||
| ) |
There was a problem hiding this comment.
It would make sense for this error message to directly link to the migration documentation, but since our docs aren't versioned this seems impossible to do currently.
|
tried the psql seeded database migration locally. I think we should output some statistics This message is correct "Database initialised successfully!" but in a migration step it better should tell "Database migration successfully!". The first one comes always on any start. |
There was a problem hiding this comment.
Hmm,
a mscolab database which was migrated once I can reset by the released version and it did not warn for the existing alembic_version by the PR version and it is not migrated. It crashes on a seed or reset then.
Traceback (most recent call last):
File "/home/reimar/PycharmProjects/2023/matrss/MSS/mslib/mscolab/mscolab.py", line 494, in <module>
main()
File "/home/reimar/PycharmProjects/2023/matrss/MSS/mslib/mscolab/mscolab.py", line 429, in main
handle_db_reset()
File "/home/reimar/PycharmProjects/2023/matrss/MSS/mslib/mscolab/mscolab.py", line 89, in handle_db_reset
flask_migrate.downgrade(directory=migrations.__path__[0], revision="base")
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/flask_migrate/__init__.py", line 111, in wrapped
f(*args, **kwargs)
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/flask_migrate/__init__.py", line 210, in downgrade
command.downgrade(config, revision, sql=sql, tag=tag)
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/alembic/command.py", line 449, in downgrade
script.run_env()
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/alembic/script/base.py", line 583, in run_env
util.load_python_file(self.dir, "env.py")
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/alembic/util/pyfiles.py", line 95, in load_python_file
module = load_module_py(module_id, path)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/alembic/util/pyfiles.py", line 113, in load_module_py
spec.loader.exec_module(module) # type: ignore
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<frozen importlib._bootstrap_external>", line 940, in exec_module
File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
File "/home/reimar/PycharmProjects/2023/matrss/MSS/mslib/mscolab/migrations/env.py", line 114, in <module>
run_migrations_online()
File "/home/reimar/PycharmProjects/2023/matrss/MSS/mslib/mscolab/migrations/env.py", line 108, in run_migrations_online
context.run_migrations()
File "<string>", line 8, in run_migrations
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/alembic/runtime/environment.py", line 948, in run_migrations
self.get_context().run_migrations(**kw)
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/alembic/runtime/migration.py", line 627, in run_migrations
step.migration_fn(**kw)
File "/home/reimar/PycharmProjects/2023/matrss/MSS/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py", line 30, in downgrade
with op.batch_alter_table('users', schema=None) as batch_op:
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/contextlib.py", line 144, in __exit__
next(self.gen)
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/alembic/operations/base.py", line 398, in batch_alter_table
impl.flush()
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/alembic/operations/batch.py", line 116, in flush
fn(*arg, **kw)
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/alembic/ddl/impl.py", line 343, in drop_column
self._exec(base.DropColumn(table_name, column, schema=schema))
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/alembic/ddl/impl.py", line 207, in _exec
return conn.execute(construct, multiparams)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1422, in execute
return meth(
^^^^^
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/sqlalchemy/sql/ddl.py", line 180, in _execute_on_connection
return connection._execute_ddl(
^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1533, in _execute_ddl
ret = self._execute_context(
^^^^^^^^^^^^^^^^^^^^^^
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1850, in _execute_context
return self._exec_single_context(
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1990, in _exec_single_context
self._handle_dbapi_exception(
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 2357, in _handle_dbapi_exception
raise sqlalchemy_exception.with_traceback(exc_info[2]) from e
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1971, in _exec_single_context
self.dialect.do_execute(
File "/home/reimar/Miniforge/envs/mssdev/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 919, in do_execute
cursor.execute(statement, parameters)
sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedColumn) column "profile_image_path" of relation "users" does not exist
[SQL: ALTER TABLE users DROP COLUMN profile_image_path]
(Background on this error at: https://sqlalche.me/e/20/f405)
QWidget: Must construct a QApplication before a QWidget
Abgebrochen
The released version can use the database again by
mscolab db --init
mscolab db --seed
There was a problem hiding this comment.
after dropping all tables and types from an existing database and migrating again it shows
python mslib/mscolab/mscolab.py start
WARNING:root:Couldn't import setup_saml2_backend (ImportError:'No module named 'setup_saml2_backend''), using dummy config.
INFO [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO [alembic.runtime.migration] Will assume transactional DDL.
INFO [alembic.runtime.migration] Running upgrade -> 92eaba86a92e, To version 8.3.5 - Initial migration
INFO [alembic.runtime.migration] Running upgrade 92eaba86a92e -> c171019fe3ee, To version 9.0.0
INFO [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO [alembic.runtime.migration] Will assume transactional DDL.
INFO [alembic.runtime.migration] Running upgrade c171019fe3ee -> 922e4d9c94e2, To version 10.0.0
Database initialised successfully!
WARNI [root] Couldn't import mscolab_auth (ImportError:'{No module named 'mscolab_auth'), creating dummy config.
INFO: MSS Version: 9.1.0
INFO: Python Version: 3.11.6 | packaged by conda-forge | (main, Oct 3 2023, 10:40:35) [GCC 12.3.0]
INFO: Platform: Linux-6.5.0-10043-tuxedo-x86_64-with-glibc2.35 (('64bit', 'ELF'))
INFO: Launching MSColab Server
now also with recreation of an empty database I get the migration information. |
I don't quite understand. You created a database with this PRs code, then went back to the released MSColab 9.1.0, and it couldn't deal with that database? Of course that is unsupported and will in all likelihood break.
Yes, (re-)creating the database is applying all migrations in order.
That's what MSS/mslib/mscolab/app/__init__.py Lines 67 to 76 in 9bd5eac Databases that were manually migrated with the previous instructions don't have this set so their naming conventions will be the databases default ones. As you can read up here, it would be very tedious to write proper migrations from those database-specific names to our new naming scheme, and with SQLite I am pretty sure that it would be outright impossible because e.g. unique constraints simply don't have a name there and can not be referenced in any way. I noticed this because the previous instructions produced a migration script for SQLite that did not include dropping the unique constraint on passwords, so the previous instructions wouldn't actually produce a SQLite database in sync with the model definition. Simply creating a new database and copying all data over was the only feasible and portable migration path I could think of. |
|
At this point one can also do a database engine migration. |
True, I didn't even think of that. A nice side-effect. |
|
I have a problem on our server it looks like: |
With this flask-migrate is used on application setup to automatically migrate the configured database to the latest revision shipped with MSColab. This will make manual migrations in production environments unnecessary in the future. Also included is an upgrade path from databases which were previously manually managed. This entails generating a new database and copying over all data.
|
@ReimarBauer it was copying the previous databases alembic_version table over as well, which in your case contained an unknown revision from following the old manual migration instructions. I didn't catch that before since the tests simply dropped that table. It should be fixed now, can you try again? |
later today |
|
looks good so far. Testing at the next day ;) |
Purpose of PR?:
Fixes #2416.
Does this PR introduce a breaking change?
If the changes in this PR are manually verified, list down the scenarios covered::
Additional information for reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs
Does this PR results in some Documentation changes?
If yes, include the list of Documentation changes
Checklist:
<type>: <subject>