Make schema name optional about alembic_utils HOT 10 CLOSED

I'm familiar with postgres search_path but I don't follow how it being set interacts with requiring an explicit schema in ReplaceableEntity.

I've seen sqlalchemy's optional schema cause enough difficult-to-follow behavior that making it required in alembic_utils was an intentional design choice.

If you have a concrete example of where the value of search_path causes unexpected behavior with an explicit ReplaceableEntity.schema I'm happy to take a look, but if the goal is convenience, I prefer to keep it as required.

from alembic_utils.

The goal was adaptability. I can deploy my migrations onto a PG installation where the DBA has set the search_path for the account, and everything runs correctly except for the ReplaceableEntity objects, which have a hard-coded schema name.

from alembic_utils.

That's a fair use case but niche enough that I don't think its worth handling all the consequences of making schemas optionals to support it

If you make a connection from env.py to read the search path you could set the schema dynamically

# env.py
schema_name: str = conn.execute(....

MyPGView.schema = schema_name

register_entities([MyPGView])

but the same migration needs to be applied in multiple schemas dynamically that will still require manual edits to the autogenerate output

from alembic_utils.

I ran into a related issue; I have a few tests that use do a Base.metadata.drop_all() on cleanup. Since I startd explicitly defining some schema's, this is broken because SQLAlchemy tries to delete items in the wrong order, leading to conflicts. I can manually override the drop order, but this is quite a hassle.

from alembic_utils.

@tdamsma this is a different issue than Martijn described

alembic_utils does not register entities against a sqlalchemy Metadata instance so Base.metadata.drop_all() would not interact.

If you have e.g. defined a PGView that refers to a sqlalchemy table, its possible that the dependency is preventing the drops from succeeding if that method does not issue a DROP ... CASCADE.

But if that was the issue, changing the table drop order would not have resolved it.

If you think its worth documenting for other users, please open a new issue and provide a reproducible example and stacktrace.

Unrelated to this problem, if you're dropping and recreating tables between tests to make sure the database is in an empty state, you should consider running tests in a transaction that rolls back after each test. It is significantly faster and parallel safe if you ever need to parallelize a large test suite.

unittest transactional test setup
pytest transactional test setup

from alembic_utils.

I agree using transactions is a much better pattern, but the test suite is not fully migrated (yet?).

I think I was not entirely clear what I meant, but because alembic-utils requires explicit schema's, my other tables that are operated on by PGFunctions now also need explicit schema's as to not throw off alembic. And then that (maybe?) causes the resolution order of of dependencies in SQLAlchemy to break. Not 100% sure, but I strongly suspect some part of Alembic and/or SQLAlchemy does not like it if half the tables have the schema name implicitly set to public, and the other half has it set explicitly. Now I have the options:

• Use explicit schema names everywhere
• Deal with the fall out (manually adding schema names to alembic migrations, manually tune Base.metadata.drop_all() order, etc )
• Not use explicit public schema calls anywhere

I would prefer the last, so that is how it related to this issue

from alembic_utils.

Got it
I'm happy to help you get set up but I won't be able to assist without a concrete reproducible example.

If you'd like explicitly set a schema="public" for all tables associated with your Base.metadata you can set it globally on the metadata instance https://docs.sqlalchemy.org/en/14/core/metadata.html#sqlalchemy.schema.MetaData rather than adding it to __tableargs__ for every table.

e.g.

from sqlalchemy import MetaData
from sqlalchemy.orm import declarative_base

metadata = MetaData(schema="public")
Base = declarative_base(metadata)

class MyTable(Base):
    ...

from alembic_utils.

Thanks for the offer! I didn't know about the option to set a schema globally, learned something already. I decided to give option 3 a try, see #54

I browsed through the issues, and as #41 also is about resolution order it might be related to the behaviour I ran into that when a single table is referred to as both some_table and public.some_table this sees to confuse SQLAlchemy/Alembic somehow

from alembic_utils.

Resolution order in #44 refers to the order in which entities must be rendered in a migration upgrade/downgrade block for the migration to succeed. It does not relate to runtime DDL statements.

I appreciate the effort required to open a PR, but as I noted previously, requiring a schema was a deliberate design decision. I will not consider any changes until I have a reproducible example so that I can understand the issue you're facing.

from alembic_utils.

Well it's your library and I'm happy you made it nonetheless. I can't create a decent minimal example detailing the wonkiness I experienced in having mixed optional and default schema use I experienced with Alembic/SQLALchemy, so I understand ,my case is not very convincing. I just want to make the point that SQLAlchemy and Alembic do not require schema's to be set explicitly, and this extension to alembic does. This design decision creates (for me at least) some ripple effects when adding a few triggers to and existing applications with alembic-utils. It is not so important to me as want to figure out the exacpt root cause, and it may very well be that there is a deeper problem in my application but that is not the point; I just wanted to add a trigger, and now I had make some tweaks to seemingly unrelated test-fixtures and table definitions.

Perhaps you can better document this design decision as it is a bit unexpected for an extension to alembic to pose additional contraints (explicit schema's) not really related to the core added functionality (support for migratable triggers and functions.

from alembic_utils.