-
Notifications
You must be signed in to change notification settings - Fork 29
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
Alembic generated migrations support #229
Comments
Problem with primary keys in Alembic only generates migration files, while |
@louimet Would you be able able to take a look at this issue and let us know the error you are facing? |
Working on v3.19.0, I used the following declarative class to generate two migrations. The first creates the 'test' table with only the 'id' column, the second adds the 'name column.
Running the first migration works fine. But running the second migration gives :
Note that examining the equivalent DDL for the alembic_version table in the database gives this: So it would seem that the version_num column is tagged as the primary key even though we specify version_table_pk=False in our env.py as prescribed. |
@louimet, hm-m, let's think... As far as I understand, you've tried to do a first migration, it failed. Than you've added this I suppose, in this case it'll not work, because |
See for more details: #229 (comment)
@IlyaFaer No, version_table_pk was False from the start. I thought that might be it in our actual database, that we might have added that subsequently. That's why I started from scratch, creating a new database, to make sure that I had that setting with the correct value from the start. And I still get that error. I just checked again :
The error doesn't obviously point to a problem with the primary key in the alembic_version table, rather mentionning a "concurrent modification". It could be a complete red herring, but looking at the details, I'm a bit confused. It's not obvious that the shown "equivalent DDL" is the actual DDL that was used to create the table. It mentions PRIMARY KEY, but it's not clear whether the table has one or not, as shown by the following comparison. Also, using the information mentioned here, I looked up the details of the database, and it looks like the alembic_version table does indeed have a primary key constraint, but no column attached to it. Again, maybe the PK is not the culprit here, but then I don't see what could be the "concurrent modification" that's making the transaction fail. Also note that running both migrations in a fresh new database in a single 'alembic upgrade head' works perfectly fine. That really points to some problem during the update in the alembic_version table. Here's my complete env.py file.
|
@louimet, I was able to reproduce it - thanks for the detailed description! I think I understand the problem now (you can scroll to the bottom of the post for the solution). @ansh0l, @asthamohta, this probably should be a question for the Spanner backend team. Logging all the operations, made by Alembic during migration, I see this: SELECT alembic_version.version_num FROM alembic_version
INFO [alembic.runtime.migration] Running upgrade 7f0ef95eb985 -> 6c6c3c5f73a2, second_mig
CREATE TABLE test2 (
id STRING(3) NOT NULL
) PRIMARY KEY (id)
UPDATE alembic_version SET version_num='6c6c3c5f73a2' WHERE alembic_version.version_num = '7f0ef95eb985' There are no concurrent updates, but Here should be noted that the migration is actually applied, and the table The weird moment is this: if these operations are executed separately and manually, they all are passing fine. Overthinking it a bit, I decided to try to put Spanner connections into AUTOCOMMIT mode by default, and migration passed fine: And here a couple of questions appear:
@louimet, looks like the solution for your case is to make Alembic work with Spanner in autocommit mode. Fortunately, Alembic already have a tool for cases like ours, see the docs. I tried it locally, seems to be solving the problem: Let me know if it solves your problem. If yes, I'll update the repo docs to mention this case and the solution. |
See for more details: #229 (comment)
Yes, it seems to solve the problem.
Running it gives this:
The table is created properly, and the alembic_version table is updated with the correct version number, so everything looks fine. However, since Alembic's autocommit is based on SQLAlchemy's autocommit, which is going to be deprecated in SqlAlchemy 2.0, I'm not sure this would be a viable long-term solution, it will at least require some tweaking (but we can use that in the meantime). On the whole though, isn't that a hint that there's an underlying problem in the way transactions are segmented? |
Well, we're not the only database, which requires autocommit during migrations (and autocommit on the whole), so there will be solution for all of us in SQLAlchemy 2.0.
I'd say rules of transactions abortion are too strict for now. In this particular case it seems to be safe to run DDL and continue transaction, but it's still aborted. We brought this to responsible engineers' attention, will see what they'll say. |
Cool, thanks! |
This issue and this solution should be added to the main doc. I spent more than a day struggling with this problem and just when i was about to give up, i thought to take a look at closed issues and here it is. The autocommit worked like charm with a warning. Please add this to the main docs. It will save a lot of time for others. |
The issue happened to me as well but it was failing in alembic_version migration table creation step. It wasn't able to create migration version table due to lack of Primary key defined. Running the following directly on psql using PGAdapter failed with ERROR: Primary key must be defined for table "alembic_version". CREATE TABLE alembic_version (
version_num VARCHAR(32) NOT NULL
) I understand that Spanner require primary key and primary key cannot be updated. That's why the env.py config on alembic needs to be set to with connectable.connect() as connection:
context.configure(
connection=connection,
target_metadata=target_metadata,
version_table_pk=False, # don't use primary key in the versions table
) But this just got rid of the PRIMARY syntax from the alembic_version table creation step, not assigning a new required primary key. |
Check how limitations are handled by generated migrations:
The text was updated successfully, but these errors were encountered: