sqlalchemy-bot / test_alembic_1

0 stars 0 forks source link

nosuchtableerror on autogenerate (was: Feature proposal: Choose the schema to use for the migrations ) #493

Open sqlalchemy-bot opened 6 years ago

sqlalchemy-bot commented 6 years ago

Migrated issue, originally created by jean soula (jeansoula)

Hello !

I was struggling to generate revision for one of my company's project and the workaround was to modify some code in autogenerate/compare.py.

Our changes can be found in the attached file (patch format).

Context

My python app needs to manage data in a specific postgresql schema (let's call it "custom_schema"). Some tables in this schema have foreign key to other tables in schema "public" (and I don't want to apply migrations on the schema "public").

In order to achieve this, I added SET local search_path to custom_schema in my env.py file.

While running alembic revision --autogenerate, sqlalchemy.exc.NoSuchTableError was raised. After some investigations I realized that code in autogenerate/compare.py was comparing the state "custom_schema" with the "public" schema. Which made the migration crash (Tables in "public" were marked as "removed").

Fix

The proposed patch fixes this behavious by adding an option "target_schema" to context.configure in env.py.

I'll be happy to discuss about this proposal.

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

counterproposal: set http://alembic.zzzcomputing.com/en/latest/api/runtime.html#alembic.runtime.environment.EnvironmentContext.configure.params.include_schemas so that autogenerate considers all schemas, then use http://alembic.zzzcomputing.com/en/latest/api/runtime.html#alembic.runtime.environment.EnvironmentContext.configure.params.include_object to tune the list of schemas / tables that you actually care about. this can be added as a recipe to http://alembic.zzzcomputing.com/en/latest/cookbook.html .

motivation: there should be only one way to do it, more flags/switches adds to user confusion, testing burden, brittleness

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

counterproposal #2: have the include_schemas parameter also be passed as a list that includes the schema names to include. motivation here is that the include_object() routine is passed each Table object found in every schema; for a system that has hundreds of schemas this will waste a lot of time.

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

but really, the include_object routine in my first comment is what you need here

sqlalchemy-bot commented 6 years ago

jean soula (jeansoula) wrote:

Hi,

I'm trying to do what you said, add include_schemas=True, include_object=include_object but I have always the error: sqlalchemy.exc.NoSuchTableError: TableName (in public schema)

Cause the filter include_object on default schema Public will be apply after the line inspector.reflecttable(t, None) which returns to me the error above.

autogenerate/compare.py at line 137:

if not exists:
    event.listen(
        t,
        "column_reflect",
        autogen_context.migration_context.impl.
        _compat_autogen_column_reflect(inspector))
    inspector.reflecttable(t, None)

    if autogen_context.run_filters(t, tname, "table", True, None):
         .....
sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

please illustrate exact table models that can reproduce this issue as well as a complete stack trace, as that's not expected. Additionally, when using this technique, ensure that your search_path is set to "public" only, see the notes at http://docs.sqlalchemy.org/en/latest/dialects/postgresql.html#remote-schema-table-introspection-and-postgresql-search-path for the choices SQLAlchemy has made on this and their implications.

sqlalchemy-bot commented 6 years ago

jean soula (jeansoula) wrote:

Hi,

I did what you recommend:

schema_name = target_metadata.schema

def include_object(object, name, type_, reflected, compare_to):
    if object.schema != schema_name:
        return False
    else:
        return True
with connectable.connect() as connection:
        connection.execute(f'CREATE SCHEMA IF NOT EXISTS {schema_name};')
        context.configure(
            connection=connection,
            target_metadata=target_metadata,
            version_table_schema=schema_name,
            include_schemas=True,
            include_object=include_object,
        )

And it works like a charm! :)

Thank you very much for your help

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

it's easy, right? that's why I didn't want to add another flag that does ten percent of the same thing. confirm you just needed to leave search path at it's default?

sqlalchemy-bot commented 6 years ago

Changes by Michael Bayer (zzzeek):