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

ORM session creates a subtransaction on get_bind()? this interferes w/ things ? #369

Closed
sqlalchemy-bot opened this Issue Apr 17, 2016 · 10 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Apr 17, 2016

Migrated issue, originally created by chris7 (@chris7)

I am trying to run a trivial migration, and alembic will migrate the database, but is not updating the database version. To update alembic_version, it requires me to run the migration again.

Here's the script I am running:

def upgrade():
  op.add_column(
    'peaks',
    sa.Column('rti', sa.Float(), nullable=True),
  )

And the output of a full migration:

alembic upgrade head
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade 1 -> 2, Add peakgroups
INFO  [alembic.runtime.migration] Running upgrade 2 -> 3, peakgroup_feature_reference
INFO  [alembic.runtime.migration] Running upgrade 3 -> 4, feature_to_peakgroup
INFO  [alembic.runtime.migration] Running upgrade 4 -> 5, remove feature peaks
INFO  [alembic.runtime.migration] Running upgrade 5 -> 6, add retention time indices
$ alembic current
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
5

The database at this point has the 'rti' column, so it was successful but the alembic_version table was never updated. I can run the migration again, and it will stamp the database with 6.

running alembic v. 0.8.6 & SQLAlchemy 1.0.12. The database is a sqlite file.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 17, 2016

chris7 (@chris7) wrote:

I discovered why this did not work. In migration 4, I created a session object, ran some SQL inserts, and then left without calling session.commit (since I wasn't actually using the ORM).:

connection = op.get_bind()
Session = sa.orm.sessionmaker()
session = Session(bind=connection)
# connection.execute stuff, but no session.commit()

Why this manifested itself in migration 6, I have no idea!

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 17, 2016

Michael Bayer (@zzzeek) wrote:

it's not possible for "alembic upgrade" to "stamp" from revision 5 to 6 without also running the contents of migration number 5, which would fail if that column were already in place (unless perhaps you're using batch mode, not indicated above). I'm not really sure that working with the session in a particular migration really has any affect on anything either.

Overall I can't do anything here without more detail, at least the full SQL log of each migration would be helpful, as well migration files which reproduce the behavior.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 18, 2016

chris7 (@chris7) wrote:

It was a smaller example. I have a helper function that checks if the column exists (because otherwise as you said, it fails):

def column_exists(op, table_name, column_name):
  connection = op.get_bind()
  return any(i[1] == column_name for i in connection.execute('PRAGMA table_info({});'.format(table_name)))
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 18, 2016

chris7 (@chris7) wrote:

It wasn't in migration 5. That was just a red herring that it was stuck at 5. It's actually migration 4, which looked like:

def upgrade():
  connection = op.get_bind()
  Session = sa.orm.sessionmaker()
  session = Session(bind=connection)
  ...
  # peakgroups and peaks are lists of ORM objects
  connection.execute(
    PeakGroup.__table__.insert(),
    peakgroups
  )
  connection.execute(
    peakgroups_peak_table.insert(),
    peakgroup_peak_map
  )
  feature_table = Feature.__table__
  connection.execute(
    sa.update(
      feature_table, values={feature_table.c.peakgroup_id: feature_table.c.id}
    )
  )

My error was NOT calling session.commit(). If I add session.commit() to the end, all is well in the world.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 18, 2016

Michael Bayer (@zzzeek) wrote:

yeah OK if that's actually happening that should perhaps be improved.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 18, 2016

Changes by Michael Bayer (@zzzeek):

  • changed title from "Alembic_version out of sync with database" to "ORM session creates a subtransaction on get_bind()"
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 23, 2017

Michael Bayer (@zzzeek) wrote:

there's no issue if transactional_ddl is turned on because there's a transaction enclosing the whole thing.

for transactional_ddl=False then we're relying on SQLAlchemy autocommit so let's see if we can get a warning in there.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 23, 2017

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 23, 2017

Michael Bayer (@zzzeek) wrote:

Detect open transaction at end of migration w/ transactional_ddl=False

A :class:.CommandError is now raised when a migration file opens
a database transaction and does not close/commit/rollback, when
the backend database or environment options also specify transactional_ddl
is False. When transactional_ddl is not in use, Alembic doesn't
close any transaction so a transaction opened by a migration file
will cause the following migrations to fail to apply.

Change-Id: I7a9baf18837deb193d9ddc6813955909484d4945
Fixes: #369

3f070ea

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 23, 2017

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment