Skip to content
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 current command shouldn't mutate the database #694

Closed
edigaryev opened this issue May 14, 2020 · 6 comments
Closed

alembic current command shouldn't mutate the database #694

edigaryev opened this issue May 14, 2020 · 6 comments
Labels
bug Something isn't working execution model

Comments

@edigaryev
Copy link
Contributor

I have a case where two types of Kubernetes applications are using the same database schema. Before they are started, the following happens:

  • multiple writers run alembic current in loop waiting for migrations to be applied
  • single reader runs alembic upgrade head before it starts

This "locking" mechanism causes problems due to how alembic current is implemented. It's description says:

Display the current revision for a database.

However besides displaying the revision it also tries to create an empty alembic_version table, which may cause race conditions when multiple alembic current commands are being run in parallel:

[...]
sqlalchemy.exc.IntegrityError: (psycopg2.errors.UniqueViolation) duplicate key value violates unique constraint "pg_type_typname_nsp_index"
DETAIL:  Key (typname, typnamespace)=(alembic_version, 2200) already exists.

In fact, it does not make sense to create anything since the output after creation looks like this (the same amount of information could be provided without creating anything):

$ alembic current
INFO  [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO  [alembic.runtime.migration] Will assume transactional DDL.

It would be nice if alembic current would implement an early exit if alembic_version table does not exist.

@edigaryev edigaryev added the requires triage New issue that requires categorization label May 14, 2020
@zzzeek zzzeek added bug Something isn't working execution model and removed requires triage New issue that requires categorization labels May 14, 2020
@zzzeek
Copy link
Member

zzzeek commented May 14, 2020

agree, the creation of the table is part of a fixed workflow and some effort would need to be taken to figure out how to break this cycle in the case of a command like "current". some flag would need to be passed thorugh. any interest on working on a PR ?

@zzzeek
Copy link
Member

zzzeek commented May 14, 2020

this should do it :

diff --git a/alembic/command.py b/alembic/command.py
index 7d19e3c..34c1075 100644
--- a/alembic/command.py
+++ b/alembic/command.py
@@ -511,7 +511,7 @@ def current(config, verbose=False, head_only=False):
 
         return []
 
-    with EnvironmentContext(config, script, fn=display_version):
+    with EnvironmentContext(config, script, fn=display_version, dont_mutate=True):
         script.run_env()
 
 
diff --git a/alembic/runtime/migration.py b/alembic/runtime/migration.py
index 48408a4..5c8590d 100644
--- a/alembic/runtime/migration.py
+++ b/alembic/runtime/migration.py
@@ -497,7 +497,9 @@ class MigrationContext(object):
         else:
             heads = self.get_current_heads()
 
-            if not self.as_sql and not heads:
+            dont_mutate = self.opts.get("dont_mutate", False)
+
+            if not self.as_sql and not heads and not dont_mutate:
                 self._ensure_version_table()
 
         head_maintainer = HeadMaintainer(self, heads)

@edigaryev
Copy link
Contributor Author

any interest on working on a PR ?

Yes, but it seems that you've already nailed it :)

I've opened a PR (#695) and added a simple test for this behavior.

@sqla-tester
Copy link
Collaborator

Nikolay Edigaryev has proposed a fix for this issue in the master branch:

Ensure "alembic current" won't unnecessarily mutate the database https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/1970

@dogweather
Copy link

dogweather commented Nov 14, 2023

I'm getting this error in CI (GitHub) with v. 1.12.1 inside a docker container. Anyone know if it was resolved? My workaround:

docker compose exec web alembic current
docker compose exec web alembic upgrade head
docker compose exec web alembic current

@CaselIT
Copy link
Member

CaselIT commented Nov 14, 2023

Can you open a new issue. Your workaround will create the table so I'm not sure it's the same issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working execution model
Projects
None yet
Development

No branches or pull requests

5 participants