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

Using as_sql=True and autogenerate results in exception in SQLAlchemy and alembic #266

Closed
sqlalchemy-bot opened this Issue Jan 14, 2015 · 8 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Jan 14, 2015

Migrated issue, originally created by Johannes Erdfelt (@jerdfelt)

I tested this using the trunk versions of both alembic and SQLAlchemy.

Using a simple test program with as_sql=True, an exception occurs in SQLAlchemy:

Traceback (most recent call last):
  File "test_as_sql.py", line 15, in <module>
    print compare_metadata(context, metadata)
  File "/home/johannes/openstack/alembic/alembic/autogenerate/api.py", line 116, in compare_metadata
    object_filters, include_schemas)
  File "/home/johannes/openstack/alembic/alembic/autogenerate/api.py", line 216, in _produce_net_changes
    inspector = Inspector.from_engine(connection)
  File "/home/johannes/openstack/sqlalchemy/lib/sqlalchemy/engine/reflection.py", line 135, in from_engine
    return Inspector(bind)
  File "/home/johannes/openstack/sqlalchemy/lib/sqlalchemy/engine/reflection.py", line 109, in __init__
    bind.connect().close()
AttributeError: 'NoneType' object has no attribute 'close'

It appears that the mock engine used when as_sql is True doesn't return self from the connect() method (and also doesn't have a close() method at all).

If I add simple connect() and close() methods in MockConnection, then I run into another exception:

Traceback (most recent call last):
  File "test_as_sql.py", line 15, in <module>
    print compare_metadata(context, metadata)
  File "/home/johannes/openstack/alembic/alembic/autogenerate/api.py", line 116, in compare_metadata
    object_filters, include_schemas)
  File "/home/johannes/openstack/alembic/alembic/autogenerate/api.py", line 219, in _produce_net_changes
    default_schema = connection.dialect.default_schema_name
AttributeError: 'MySQLDialect_mysqldb' object has no attribute 'default_schema_name'

Looking at the code, it seems like the connection passed in gets discarded in lieu of the mock connection. So it makes me think that the autogenerate code would never work with as_sql=True.

I've worked around this (or maybe this is what I should be doing all along) by creating two different contexts. One with as_sql=False for alembic.autogenerate and then one with as_sql=True for alembic.operations.


Attachments: test_as_sql.py

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 14, 2015

Michael Bayer (@zzzeek) wrote:

So that's a small issue that can be fixed, but using autogenerate with sql=True makes no sense. what's that about ?

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 14, 2015

Johannes Erdfelt (@jerdfelt) wrote:

Well, I created one context that I used for autogenerate and then for operations. It seemed like it should work at the time! :)

I certainly can live with creating two contexts as what should be done. If so, would you be against a small patch that raises a clearer exception if as_sql=True is used with autogenerate? I guess an alternative would be to ignore as_sql for autogenerate, but that might be confusing if people expect as_sql to never contact the database.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 14, 2015

Michael Bayer (@zzzeek) wrote:

it's better that it raises. im not sure if some folks are trying to do something with autogenerate and --sql, however.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 16, 2015

Michael Bayer (@zzzeek) wrote:

will pull in your PR when I get a chance

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 16, 2015

Changes by Michael Bayer (@zzzeek):

  • set milestone to "fasttrack"
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 19, 2015

Michael Bayer (@zzzeek) wrote:

Raise exception if autogenerate is tried with as_sql=True

This configuration is nonsensical since autogenerate needs to query
the database for schema information.

Fixes issue #266

9e24654

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 19, 2015

Michael Bayer (@zzzeek) wrote:

  • changelog for #266
  • use exception fixture
  • look directly at context.as_sql as that's where
    the "sql mode" is most authoritative
  • fixes #266

4c2f825

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 19, 2015

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