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

ValueError: Constraint must have a name in alembic 1.10.0 #1195

Open
harupy opened this issue Mar 6, 2023 · 16 comments
Open

ValueError: Constraint must have a name in alembic 1.10.0 #1195

harupy opened this issue Mar 6, 2023 · 16 comments

Comments

@harupy
Copy link
Contributor

harupy commented Mar 6, 2023

Describe the bug

ValueError: Constraint must have a name in alembic 1.10.0.

Expected behavior

Migration succeeds.

To Reproduce
Please try to provide a Minimal, Complete, and Verifiable example, with the migration script and/or the SQLAlchemy tables or models involved.
See also Reporting Bugs on the website.

I'm trying to create a minimal reproducer now, but the migration script that caused the error looks like this:

"""Update run status constraint with killed

Revision ID: cfd24bdc0731
Revises: 89d4b8295536
Create Date: 2019-10-11 15:55:10.853449

"""
import alembic
from alembic import op
from packaging.version import Version
from sqlalchemy import CheckConstraint, Enum

from mlflow.entities import RunStatus, ViewType
from mlflow.entities.lifecycle_stage import LifecycleStage
from mlflow.store.tracking.dbmodels.models import SqlRun, SourceTypes

# revision identifiers, used by Alembic.
revision = "cfd24bdc0731"
down_revision = "2b4d017a5e9b"
branch_labels = None
depends_on = None

old_run_statuses = [
    RunStatus.to_string(RunStatus.SCHEDULED),
    RunStatus.to_string(RunStatus.FAILED),
    RunStatus.to_string(RunStatus.FINISHED),
    RunStatus.to_string(RunStatus.RUNNING),
]

new_run_statuses = [*old_run_statuses, RunStatus.to_string(RunStatus.KILLED)]

# Certain SQL backends (e.g., SQLite) do not preserve CHECK constraints during migrations.
# For these backends, CHECK constraints must be specified as table arguments. Here, we define
# the collection of CHECK constraints that should be preserved when performing the migration.
# The "status" constraint is excluded from this set because it is explicitly modified
# within the migration's `upgrade()` routine.
check_constraint_table_args = [
    CheckConstraint(SqlRun.source_type.in_(SourceTypes), name="source_type"),
    CheckConstraint(
        SqlRun.lifecycle_stage.in_(LifecycleStage.view_type_to_stages(ViewType.ALL)),
        name="runs_lifecycle_stage",
    ),
]


def upgrade():
    # In alembic >= 1.7.0, `table_args` is unnecessary since CHECK constraints are preserved
    # during migrations.
    table_args = (
        [] if Version(alembic.__version__) >= Version("1.7.0") else check_constraint_table_args
    )
    with op.batch_alter_table("runs", table_args=table_args) as batch_op:
        # Transform the "status" column to an `Enum` and define a new check constraint. Specify
        # `native_enum=False` to create a check constraint rather than a
        # database-backend-dependent enum (see https://docs.sqlalchemy.org/en/13/core/
        # type_basics.html#sqlalchemy.types.Enum.params.native_enum)
        batch_op.alter_column(
            "status",
            type_=Enum(
                *new_run_statuses,
                create_constraint=True,
                native_enum=False,
            ),
            existing_type=Enum(
                *old_run_statuses,
                create_constraint=True,
                native_enum=False,
                name="status",
            ),
        )


def downgrade():
    # Omit downgrade logic for now - we don't currently provide users a command/API for
    # reverting a database migration, instead recommending that they take a database backup
    # before running the migration.
    pass

Error

Traceback (most recent call last):
  File "a.py", line 5, in <module>
    with mlflow.start_run():
  File "/home/haru/Desktop/repositories/mlflow/mlflow/tracking/fluent.py", line 278, in start_run
    client = MlflowClient()
  File "/home/haru/Desktop/repositories/mlflow/mlflow/tracking/client.py", line 69, in __init__
    self._tracking_client = TrackingServiceClient(final_tracking_uri)
  File "/home/haru/Desktop/repositories/mlflow/mlflow/tracking/_tracking_service/client.py", line 51, in __init__
    self.store
  File "/home/haru/Desktop/repositories/mlflow/mlflow/tracking/_tracking_service/client.py", line 55, in store
    return utils._get_store(self.tracking_uri)
  File "/home/haru/Desktop/repositories/mlflow/mlflow/tracking/_tracking_service/utils.py", line 217, in _get_store
    return _tracking_store_registry.get_store(store_uri, artifact_uri)
  File "/home/haru/Desktop/repositories/mlflow/mlflow/tracking/_tracking_service/registry.py", line 39, in get_store
    return self._get_store_with_resolved_uri(resolved_store_uri, artifact_uri)
  File "/home/haru/Desktop/repositories/mlflow/mlflow/tracking/_tracking_service/registry.py", line 49, in _get_store_with_resolved_uri
    return builder(store_uri=resolved_store_uri, artifact_uri=artifact_uri)
  File "/home/haru/Desktop/repositories/mlflow/mlflow/tracking/_tracking_service/utils.py", line 150, in _get_sqlalchemy_store
    return SqlAlchemyStore(store_uri, artifact_uri)
  File "/home/haru/Desktop/repositories/mlflow/mlflow/store/tracking/sqlalchemy_store.py", line 134, in __init__
    mlflow.store.db.utils._initialize_tables(self.engine)
  File "/home/haru/Desktop/repositories/mlflow/mlflow/store/db/utils.py", line 82, in _initialize_tables
    _upgrade_db(engine)
  File "/home/haru/Desktop/repositories/mlflow/mlflow/store/db/utils.py", line 206, in _upgrade_db
    command.upgrade(config, "heads")
  File "/home/haru/miniconda3/envs/mlflow-dev-env/lib/python3.8/site-packages/alembic/command.py", line 378, in upgrade
    script.run_env()
  File "/home/haru/miniconda3/envs/mlflow-dev-env/lib/python3.8/site-packages/alembic/script/base.py", line 576, in run_env
    util.load_python_file(self.dir, "env.py")
  File "/home/haru/miniconda3/envs/mlflow-dev-env/lib/python3.8/site-packages/alembic/util/pyfiles.py", line 94, in load_python_file
    module = load_module_py(module_id, path)
  File "/home/haru/miniconda3/envs/mlflow-dev-env/lib/python3.8/site-packages/alembic/util/pyfiles.py", line 110, in load_module_py
    spec.loader.exec_module(module)  # type: ignore
  File "<frozen importlib._bootstrap_external>", line 843, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/haru/Desktop/repositories/mlflow/mlflow/store/db_migrations/env.py", line 86, in <module>
    run_migrations_online()
  File "/home/haru/Desktop/repositories/mlflow/mlflow/store/db_migrations/env.py", line 80, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/home/haru/miniconda3/envs/mlflow-dev-env/lib/python3.8/site-packages/alembic/runtime/environment.py", line 867, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/home/haru/miniconda3/envs/mlflow-dev-env/lib/python3.8/site-packages/alembic/runtime/migration.py", line 624, in run_migrations
    step.migration_fn(**kw)
  File "/home/haru/Desktop/repositories/mlflow/mlflow/store/db_migrations/versions/cfd24bdc0731_update_run_status_constraint_with_killed.py", line 57, in upgrade
    batch_op.alter_column(
  File "/home/haru/miniconda3/envs/mlflow-dev-env/lib/python3.8/contextlib.py", line 120, in __exit__
    next(self.gen)
  File "/home/haru/miniconda3/envs/mlflow-dev-env/lib/python3.8/site-packages/alembic/operations/base.py", line 383, in batch_alter_table
    impl.flush()
  File "/home/haru/miniconda3/envs/mlflow-dev-env/lib/python3.8/site-packages/alembic/operations/batch.py", line 158, in flush
    fn(*arg, **kw)
  File "/home/haru/miniconda3/envs/mlflow-dev-env/lib/python3.8/site-packages/alembic/operations/batch.py", line 667, in add_constraint
    raise ValueError("Constraint must have a name")
ValueError: Constraint must have a name

Versions.

  • OS: Ubuntu 22.04
  • Python: 3.8
  • Alembic: 1.10.0
  • SQLAlchemy: 2.0.0
  • Database: SQLite
  • DBAPI:

Additional context

The migration script works fine in alembic 1.9.4.

Have a nice day!

@harupy harupy added the requires triage New issue that requires categorization label Mar 6, 2023
@harupy
Copy link
Contributor Author

harupy commented Mar 6, 2023

Seems related to 714b744

@harupy
Copy link
Contributor Author

harupy commented Mar 6, 2023

The error occurs here:

def add_constraint(self, const: Constraint) -> None:
if not constraint_name_defined(const.name):
raise ValueError("Constraint must have a name")

print(const) prints out:

CheckConstraint(<sqlalchemy.sql.elements.BinaryExpression object at 0x7f9be5fb48e0>, name=<_NoneName.NONE_NAME: 0>, table=Table('runs', MetaData(), Column('status', Enum('SCHEDULED', 'FAILED', 'FINISHED', 'RUNNING', 'KILLED', native_enum=False, create_constraint=True), table=<runs>), schema=None), _create_rule=<sqlalchemy.util.langhelpers.portable_instancemethod object at 0x7f9be5fc0300>, _type_bound=True)

@harupy
Copy link
Contributor Author

harupy commented Mar 6, 2023

diff --git a/mlflow/store/db_migrations/versions/cfd24bdc0731_update_run_status_constraint_with_killed.py b/mlflow/store/db_migrations/versions/cfd24bdc0731_update_run_status_constraint_with_killed.py
index beb02a2d3..89a207576 100644
--- a/mlflow/store/db_migrations/versions/cfd24bdc0731_update_run_status_constraint_with_killed.py
+++ b/mlflow/store/db_migrations/versions/cfd24bdc0731_update_run_status_constraint_with_killed.py
@@ -60,6 +60,7 @@ def upgrade():
                 *new_run_statuses,
                 create_constraint=True,
                 native_enum=False,
+                name="status",
             ),
             existing_type=Enum(
                 *old_run_statuses,

fixed the error.

@harupy harupy closed this as completed Mar 6, 2023
@CaselIT
Copy link
Member

CaselIT commented Mar 6, 2023

this seems a regression. Let's keep open for now, until we investigate

@CaselIT CaselIT reopened this Mar 6, 2023
@harupy
Copy link
Contributor Author

harupy commented Mar 6, 2023

@CaselIT Got it!

@CaselIT
Copy link
Member

CaselIT commented Mar 6, 2023

thanks for reporting

@ts-accessio
Copy link

We got the same problem:
Our migration:

def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    with op.batch_alter_table('movement_movement', schema=None) as batch_op:
        batch_op.add_column(sa.Column('is_anonymized', sa.Boolean(), nullable=True))
    # ### end Alembic commands ###

generates two operations
image
and in add_constraint the value of const.name is symbol('NONE_NAME'). This was no problem in 1.9.4 because there the check was if not const.name: but now it's isinstance(name, str) (inside constraint_name_defined)

@CaselIT
Copy link
Member

CaselIT commented Mar 6, 2023

thanks for reporting.

This looks like a regression. set alembic != 1.10.0 for now

@CaselIT CaselIT added bug Something isn't working regression and removed requires triage New issue that requires categorization labels Mar 6, 2023
@CaselIT
Copy link
Member

CaselIT commented Mar 6, 2023

@ts-accessio just to confirm, you are using sqlite what sqlalchemy version?

@ts-accessio
Copy link

SQLAlchemy==1.3.24
Thanks for looking into this problem

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the main branch:

distinguish between string contraint name and defined https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/4491

@kitty7c6
Copy link

Hello, I have same problems with alembic==1.10.1 (or latest)
It looks like

File "C:\Users\Erlina\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\operations\base.py", line 383, in batch_alter_table
    impl.flush()
  File "C:\Users\Erlina\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\operations\batch.py", line 159, in flush
    fn(*arg, **kw)
  File "C:\Users\Erlina\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\alembic\operations\batch.py", line 668, in add_constraint
    raise ValueError("Constraint must have a name")
ValueError: Constraint must have a name

migration file:

"""add f-key

Revision ID: d5e60373f719
Revises: 47ea41e5834e
Create Date: 2023-06-16 14:41:30.682439

"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = 'd5e60373f719'
down_revision = '47ea41e5834e'
branch_labels = None
depends_on = None


def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    with op.batch_alter_table('search_module', schema=None) as batch_op:
        batch_op.add_column(sa.Column('session_id', sa.Integer(), nullable=True))
        batch_op.create_foreign_key(None, 'searching_session', ['session_id'], ['id'])

    # ### end Alembic commands ###


def downgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    with op.batch_alter_table('search_module', schema=None) as batch_op:
        batch_op.drop_constraint(None, type_='foreignkey')
        batch_op.drop_column('session_id')

    # ### end Alembic commands ###

code:

class SearchModule(db.Model):
    sn               = db.Column(db.String(10), primary_key=True)
    status           = db.Column(db.Boolean)
    session_id       = db.Column(db.Integer, db.ForeignKey('searching_session.id'))

class Searching_session(db.Model):
    id                  = db.Column(db.Integer, primary_key=True)
    search_module       = db.relationship('SearchModule', backref='search_module', lazy='dynamic')

DB Browser for SQLite Version 3.12.2
Built for x86_64-little_endian-llp64, running on x86_64
Qt Version 5.12.8
Версия SQLite 3.35.5.

@kitty7c6
Copy link

kitty7c6 commented Jun 16, 2023

For this time I solve by changing migration file:

"""add f-key

Revision ID: d5e60373f719
Revises: 47ea41e5834e
Create Date: 2023-06-16 14:41:30.682439

"""
from alembic import op
import sqlalchemy as sa


# ###  revision identifiers, used by Alembic.
revision = 'd5e60373f719'
down_revision = '47ea41e5834e'
branch_labels = None
depends_on = None


def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    with op.batch_alter_table('search_module', schema=None) as batch_op:
        batch_op.add_column(sa.Column('session_id', sa.Integer(), nullable=True))

> this!
         batch_op.create_foreign_key(**None**, 'searching_session', ['session_id'], ['id'])
> to
        batch_op.create_foreign_key(**'session_id'**, 'searching_session', ['session_id'], ['id'])


    # ### end Alembic commands ###


def downgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    with op.batch_alter_table('search_module', schema=None) as batch_op:
        batch_op.drop_constraint(None, type_='foreignkey')
        batch_op.drop_column('session_id')

    # ### end Alembic commands ###

But in db it looks :

CREATE TABLE "search_module" (
	"sn"	VARCHAR(10) NOT NULL,
	"status"	BOOLEAN,
	"session_id"	INTEGER,
	PRIMARY KEY("sn"),
	CONSTRAINT "session_id" FOREIGN KEY("session_id") REFERENCES "searching_session"("id")
);

instead of

CREATE TABLE "search_module" (
	"sn"	VARCHAR(10) NOT NULL,
	"status"	BOOLEAN,
	"session_id"	INTEGER,
	PRIMARY KEY("sn"),
	FOREIGN KEY("session_id") REFERENCES "searching_session"("id")
);

@thornewolf
Copy link

same issue for 1.11.1

fixed using @kitty7c6 solution

@umayrh
Copy link

umayrh commented Dec 1, 2023

I'm not even creating a constraint and still running into this issue for alembic==1.13.0

        with op.batch_alter_table('employees', schema=None) as batch_op:
            batch_op.add_column(employee_number)
            batch_op.add_column(family_head_name)
            batch_op.add_column(medical_registration_number)
            batch_op.add_column(medical_registration_valid_till_date)

yields

   with op.batch_alter_table('employees', schema=None) as batch_op:
  File "~/.pyenv/versions/3.11.4/lib/python3.11/contextlib.py", line 144, in __exit__
    next(self.gen)
  File "~/Code/venv/lib/python3.11/site-packages/alembic/operations/base.py", line 375, in batch_alter_table
    impl.flush()
  File "~/Code/venv/lib/python3.11/site-packages/alembic/operations/batch.py", line 159, in flush
    fn(*arg, **kw)
  File "~/Code/venv/lib/python3.11/site-packages/alembic/operations/batch.py", line 670, in add_constraint
    raise ValueError("Constraint must have a name")
ValueError: Constraint must have a name

@Phizilion
Copy link

Phizilion commented Feb 6, 2024

I'm not even creating a constraint and still running into this issue for alembic==1.13.0

    with op.batch_alter_table('channels', schema=None) as batch_op:
        batch_op.create_unique_constraint(None, ['tg_id'])

Can confirm for alembic==1.13.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants