New column can't be added with the current migrations

Question

New column can't be added with the current migrations

ncrocfer opened this issue 3 years ago · 0 comments

A bug has been found by @henri42 in the #162 PR: it's currently impossible to add new column.

For instance:

$ cat director/migrations/versions/9d563aaa548b_add_workflows_description.py
...
def upgrade():
    op.add_column(
        "workflows",
        sa.Column("description", sa.String(255), nullable=True),
    )


def downgrade():
    op.drop_column("workflows", "description")

Applying this migration will raise a sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) no such column: workflows.description error.

Full traceback

$ flask db upgrade
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 30d6f6636351, Initial migration
INFO  [alembic.runtime.migration] Running upgrade 30d6f6636351 -> 05cf96d6fcae, Add task result
INFO  [alembic.runtime.migration] Running upgrade 05cf96d6fcae -> 3f8466b16023, Add users table
INFO  [alembic.runtime.migration] Running upgrade 3f8466b16023 -> 063ff371f2da, Add index on workflow_id in task table
INFO  [alembic.runtime.migration] Running upgrade 063ff371f2da -> 2ac615d6850b, Force varchar 255
Traceback (most recent call last):
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1245, in _execute_context
  self.dialect.do_execute(
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/engine/default.py", line 581, in do_execute
  cursor.execute(statement, parameters)
sqlite3.OperationalError: no such column: workflows.description

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

Traceback (most recent call last):
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/bin/flask", line 8, in <module>
  sys.exit(main())
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/flask/cli.py", line 985, in main
  cli.main()
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/flask/cli.py", line 579, in main
  return super().main(*args, **kwargs)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/click/core.py", line 1055, in main
  rv = self.invoke(ctx)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/click/core.py", line 1657, in invoke
  return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/click/core.py", line 1657, in invoke
  return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/click/core.py", line 1404, in invoke
  return ctx.invoke(self.callback, **ctx.params)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/click/core.py", line 760, in invoke
  return __callback(*args, **kwargs)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/click/decorators.py", line 26, in new_func
  return f(get_current_context(), *args, **kwargs)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/flask/cli.py", line 427, in decorator
  return __ctx.invoke(f, *args, **kwargs)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/click/core.py", line 760, in invoke
  return __callback(*args, **kwargs)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/flask_migrate/cli.py", line 149, in upgrade
  _upgrade(directory, revision, sql, tag, x_arg)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/flask_migrate/__init__.py", line 98, in wrapped
  f(*args, **kwargs)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/flask_migrate/__init__.py", line 185, in upgrade
  command.upgrade(config, revision, sql=sql, tag=tag)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/alembic/command.py", line 298, in upgrade
  script.run_env()
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/alembic/script/base.py", line 489, in run_env
  util.load_python_file(self.dir, "env.py")
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/alembic/util/pyfiles.py", line 98, in load_python_file
  module = load_module_py(module_id, path)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/alembic/util/compat.py", line 173, in load_module_py
  spec.loader.exec_module(module)
File "<frozen importlib._bootstrap_external>", line 783, in exec_module
File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
File "/Users/ncrocfer/Dev/deploy/celery-director/director/migrations/env.py", line 96, in <module>
  run_migrations_online()
File "/Users/ncrocfer/Dev/deploy/celery-director/director/migrations/env.py", line 90, in run_migrations_online
  context.run_migrations()
File "<string>", line 8, in run_migrations
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/alembic/runtime/environment.py", line 846, in run_migrations
  self.get_context().run_migrations(**kw)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/alembic/runtime/migration.py", line 518, in run_migrations
  step.migration_fn(**kw)
File "/Users/ncrocfer/Dev/deploy/celery-director/director/migrations/versions/2ac615d6850b_force_varchar_255.py", line 57, in upgrade
  batch_op.alter_column(
File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.8/lib/python3.8/contextlib.py", line 120, in __exit__
  next(self.gen)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/alembic/operations/base.py", line 325, in batch_alter_table
  impl.flush()
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/alembic/operations/batch.py", line 108, in flush
  batch_impl._create(self.impl)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/alembic/operations/batch.py", line 290, in _create
  op_impl._exec(
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/alembic/ddl/impl.py", line 134, in _exec
  return conn.execute(construct, *multiparams, **params)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 982, in execute
  return meth(self, multiparams, params)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/sql/elements.py", line 287, in _execute_on_connection
  return connection._execute_clauseelement(self, multiparams, params)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1095, in _execute_clauseelement
  ret = self._execute_context(
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1249, in _execute_context
  self._handle_dbapi_exception(
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1476, in _handle_dbapi_exception
  util.raise_from_cause(sqlalchemy_exception, exc_info)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/util/compat.py", line 398, in raise_from_cause
  reraise(type(exception), exception, tb=exc_tb, cause=cause)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/util/compat.py", line 152, in reraise
  raise value.with_traceback(tb)
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1245, in _execute_context
  self.dialect.do_execute(
File "/Users/ncrocfer/Dev/deploy/celery-director/venv/lib/python3.8/site-packages/sqlalchemy/engine/default.py", line 581, in do_execute
  cursor.execute(statement, parameters)
sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) no such column: workflows.description
[SQL: INSERT INTO _alembic_tmp_workflows (id, created_at, updated_at, name, project, status, payload, periodic, description) SELECT workflows.id, workflows.created_at, workflows.updated_at, workflows.name, workflows.project, workflows.status, workflows.payload, workflows.periodic, workflows.description
FROM workflows]
(Background on this error at: http://sqlalche.me/e/e3q8)

As mentionned by @henri42 in his PR we added a special syntax in the 2ac615d6850b_force_varchar_255.py migration.

This syntax was dedicated to the sqlite engine which doesn't support some manipulations. As mentioned in this article from the Flask-Migrate creator:

Changes that you make to the fields in your model or to the constraints associated with them will end up as an ALTER TABLE statement sent to the database. If you are using MySQL, Postgres or most other database servers besides SQLite, this isn't a problem. With SQLite, however, the ALTER TABLE command only supports adding or renaming columns. Any other change to columns or constraints is going to be rejected with an error.

So we use the following syntax to update the length of varchars:

with op.batch_alter_table("workflows") as batch_op:
    batch_op.alter_column(
        "name",
        existing_type=sa.VARCHAR(),
        type_=sa.String(length=255),
        existing_nullable=False,
    )

But a new bug had been raised for the sqlite engine: it was impossible to use the flask db upgrade --sql command (used to display the SQL queries). So we created a new PR #85 to use the offline mode of Alembic instead the online one (more information here).

So the new copy_from argument:

with op.batch_alter_table("workflows", copy_from=some_table) as batch_op:
    batch_op.alter_column(...)

But as said in this doc:

The above use pattern is pretty tedious and quite far off from Alembic’s preferred style of working; however, if one needs to do SQLite-compatible “move and copy” migrations and need them to generate flat SQL files in “offline” mode, there’s not much alternative.

So now we can't add new columns... 😒

I think we'll simply remove this copy_from argument and we'll reuse the online mode. The consequence will be the impossibility to generate flat SQL files using the sqlite engine. But for me this problem is not a real one:

sqlite is used when Celery Director users want to develop in local, so generate SQL files is not a real need here
when the project goes into prod, a more robust engine has to be used (like PostgreSQL or MySQL). In this case SQL files generation will be possible and users will be able to send them to their Database Administrator 👌

@ovh/gistools members, what do you think about that?