-
Notifications
You must be signed in to change notification settings - Fork 42
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
Make schema name optional #40
Comments
I'm familiar with postgres I've seen sqlalchemy's optional If you have a concrete example of where the value of |
The goal was adaptability. I can deploy my migrations onto a PG installation where the DBA has set the |
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
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 |
I ran into a related issue; I have a few tests that use do a |
@tdamsma this is a different issue than Martijn described
If you have e.g. defined a 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 |
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:
I would prefer the last, so that is how it related to this issue |
Got it If you'd like explicitly set a e.g. from sqlalchemy import MetaData
from sqlalchemy.orm import declarative_base
metadata = MetaData(schema="public")
Base = declarative_base(metadata)
class MyTable(Base):
... |
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 |
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. |
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. |
SQLAlchemy / Alembic does not set a default schema name for Postgres, nor does it require that objects set a schema. Without setting a schema, the Postgres
search_path
setting determines where objects are created.However,
alembic_utils
requires that all objects it defines have a schema set. This makes it hard to support running a migration against an environment where the schema is set withSET schema TO ...
.The text was updated successfully, but these errors were encountered: