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

Fix ORM vs migration files inconsistencies #44221

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ def downgrade():
if conn.dialect.name == "mssql":
with op.batch_alter_table("log") as batch_op:
batch_op.drop_index("idx_log_event")
batch_op.alter_column("event", type_=sa.String(30), nullable=False)
batch_op.alter_column("event", type_=sa.String(30))
batch_op.create_index("idx_log_event", ["event"])
else:
with op.batch_alter_table("log") as batch_op:
batch_op.alter_column("event", type_=sa.String(30), nullable=False)
batch_op.alter_column("event", type_=sa.String(30))
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ def upgrade():
with op.batch_alter_table("task_instance_note", schema=None) as batch_op:
batch_op.drop_constraint("task_instance_note_user_fkey", type_="foreignkey")

if op.get_bind().dialect.name == "mysql":
with op.batch_alter_table("dag_run_note", schema=None) as batch_op:
batch_op.drop_index("dag_run_note_user_fkey")

with op.batch_alter_table("task_instance_note", schema=None) as batch_op:
batch_op.drop_index("task_instance_note_user_fkey")


def downgrade():
"""Unapply Drop ab_user.id foreign key."""
Expand All @@ -53,3 +60,10 @@ def downgrade():

with op.batch_alter_table("dag_run_note", schema=None) as batch_op:
batch_op.create_foreign_key("dag_run_note_user_fkey", "ab_user", ["user_id"], ["id"])

if op.get_bind().dialect.name == "mysql":
with op.batch_alter_table("task_instance_note", schema=None) as batch_op:
batch_op.create_index("task_instance_note_user_fkey", ["user_id"], unique=False)

with op.batch_alter_table("dag_run_note", schema=None) as batch_op:
batch_op.create_index("dag_run_note_user_fkey", ["user_id"], unique=False)
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@

from __future__ import annotations

import sqlalchemy as sa
from alembic import op

from airflow.migrations.db_types import TIMESTAMP

# revision identifiers, used by Alembic.
revision = "1cdc775ca98f"
down_revision = "a2c32e6c7729"
Expand All @@ -44,14 +45,24 @@

def upgrade():
with op.batch_alter_table("dag_run", schema=None) as batch_op:
batch_op.alter_column("execution_date", new_column_name="logical_date", existing_type=sa.TIMESTAMP)
batch_op.alter_column(
"execution_date",
new_column_name="logical_date",
existing_type=TIMESTAMP(timezone=True),
existing_nullable=False,
)
with op.batch_alter_table("dag_run", schema=None) as batch_op:
batch_op.drop_constraint("dag_run_dag_id_execution_date_key", type_="unique")


def downgrade():
with op.batch_alter_table("dag_run", schema=None) as batch_op:
batch_op.alter_column("logical_date", new_column_name="execution_date", existing_type=sa.TIMESTAMP)
batch_op.alter_column(
"logical_date",
new_column_name="execution_date",
existing_type=TIMESTAMP(timezone=True),
existing_nullable=False,
)
with op.batch_alter_table("dag_run", schema=None) as batch_op:
batch_op.create_unique_constraint(
"dag_run_dag_id_execution_date_key",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ def upgrade():
op.create_table(
"backfill",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("dag_id", sa.String(length=250), nullable=True),
sa.Column("dag_id", sa.String(length=250), nullable=False),
sa.Column("from_date", airflow.utils.sqlalchemy.UtcDateTime(timezone=True), nullable=False),
sa.Column("to_date", airflow.utils.sqlalchemy.UtcDateTime(timezone=True), nullable=False),
sa.Column("dag_run_conf", sqlalchemy_jsonfield.jsonfield.JSONField(), nullable=True),
sa.Column("dag_run_conf", sqlalchemy_jsonfield.jsonfield.JSONField(), nullable=False),
sa.Column("is_paused", sa.Boolean(), nullable=True),
sa.Column("max_active_runs", sa.Integer(), nullable=False),
sa.Column("created_at", airflow.utils.sqlalchemy.UtcDateTime(timezone=True), nullable=False),
Expand All @@ -65,6 +65,10 @@ def upgrade():
sa.Column("sort_ordinal", sa.Integer(), nullable=False),
sa.PrimaryKeyConstraint("id", name=op.f("backfill_dag_run_pkey")),
sa.UniqueConstraint("backfill_id", "dag_run_id", name="ix_bdr_backfill_id_dag_run_id"),
sa.ForeignKeyConstraint(
["backfill_id"], ["backfill.id"], name="bdr_backfill_fkey", ondelete="cascade"
),
sa.ForeignKeyConstraint(["dag_run_id"], ["dag_run.id"], name="bdr_dag_run_fkey", ondelete="set null"),
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ def upgrade():
# Add 'name' column. Set it to nullable for now.
with op.batch_alter_table("dataset", schema=None) as batch_op:
batch_op.add_column(sa.Column("name", _STRING_COLUMN_TYPE))
batch_op.add_column(
sa.Column("group", _STRING_COLUMN_TYPE, default=str, server_default="", nullable=False)
)
batch_op.add_column(sa.Column("group", _STRING_COLUMN_TYPE, default="", nullable=False))
# Fill name from uri column.
with Session(bind=op.get_bind()) as session:
session.execute(sa.text("update dataset set name=uri"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
def upgrade():
"""Apply Add exception_reason and logical_date to BackfillDagRun."""
with op.batch_alter_table("backfill", schema=None) as batch_op:
batch_op.add_column(sa.Column("reprocess_behavior", sa.String(length=250), nullable=True))
batch_op.add_column(sa.Column("reprocess_behavior", sa.String(length=250), nullable=False))
with op.batch_alter_table("backfill_dag_run", schema=None) as batch_op:
batch_op.add_column(sa.Column("exception_reason", sa.String(length=250), nullable=True))
batch_op.add_column(sa.Column("logical_date", UtcDateTime(timezone=True), nullable=False))
Expand Down
16 changes: 0 additions & 16 deletions airflow/migrations/versions/0041_3_0_0_rename_dataset_as_asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,6 @@ def upgrade():
unique=False,
)

batch_op.create_foreign_key(
constraint_name="toar_asset_fkey",
referent_table="asset",
local_cols=["asset_id"],
remote_cols=["id"],
ondelete="CASCADE",
)
Copy link
Contributor Author

@ephraimbuddy ephraimbuddy Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this to a new file because, for some reason, SQLite could not create this FK in this file. I suspect it is because the asset table was renamed in this migration

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah last I had tried I ran into:

Performing upgrade to the metadata database sqlite:////root/airflow/sqlite/airflow.db
[2024-10-27T22:43:35.430+0000] {migration.py:215} INFO - Context impl SQLiteImpl.
[2024-10-27T22:43:35.431+0000] {migration.py:218} INFO - Will assume non-transactional DDL.
[2024-10-27T22:43:35.432+0000] {migration.py:215} INFO - Context impl SQLiteImpl.
[2024-10-27T22:43:35.433+0000] {migration.py:218} INFO - Will assume non-transactional DDL.
[2024-10-27T22:43:35.433+0000] {db.py:1171} INFO - Creating tables
INFO [alembic.runtime.migration] Context impl SQLiteImpl.
INFO [alembic.runtime.migration] Will assume non-transactional DDL.
INFO [alembic.runtime.migration] Running upgrade 22ed7efa9da2 -> 044f740568ec, Drop ab_user.id foreign key.
INFO [alembic.runtime.migration] Running upgrade 044f740568ec -> d0f1c55954fa, Remove SubDAGs: `is_subdag` & `root_dag_id` columns from DAG table.
INFO [alembic.runtime.migration] Running upgrade d0f1c55954fa -> 0bfc26bc256e, Rename DagModel schedule_interval to timetable_summary.
INFO [alembic.runtime.migration] Running upgrade 0bfc26bc256e -> a2c32e6c7729, Add triggered_by field to DagRun.
INFO [alembic.runtime.migration] Running upgrade a2c32e6c7729 -> 1cdc775ca98f, Drop `execution_date` unique constraint on DagRun.
INFO [alembic.runtime.migration] Running upgrade 1cdc775ca98f -> 522625f6d606, Add tables for backfill.
INFO [alembic.runtime.migration] Running upgrade 522625f6d606 -> 16cbcb1c8c36, Remove redundant index.
INFO [alembic.runtime.migration] Running upgrade 16cbcb1c8c36 -> 44eabb1904b4, Update dag_run_note.user_id and task_instance_note.user_id columns to String.
INFO [alembic.runtime.migration] Running upgrade 44eabb1904b4 -> 0d9e73a75ee4, Add name and group fields to DatasetModel.
INFO [alembic.runtime.migration] Running upgrade 0d9e73a75ee4 -> c3389cd7793f, Add backfill to dag run model.
INFO [alembic.runtime.migration] Running upgrade c3389cd7793f -> 5a5d66100783, Add AssetActive to track orphaning instead of a flag.
INFO [alembic.runtime.migration] Running upgrade 5a5d66100783 -> fb2d4922cd79, Tweak AssetAliasModel to match AssetModel after AIP-76.
INFO [alembic.runtime.migration] Running upgrade fb2d4922cd79 -> 3a8972ecb8f9, Add exception_reason and logical_date to BackfillDagRun.
INFO [alembic.runtime.migration] Running upgrade 3a8972ecb8f9 -> 05234396c6fc, Rename dataset as asset.
/usr/local/lib/python3.9/contextlib.py:126 SAWarning: WARNING: SQL-parsed foreign key constraint '('alias_id', 'asset_alias', 'id')' could not be located in PRAGMA foreign_keys for table asset_alias_asset
/usr/local/lib/python3.9/contextlib.py:126 SAWarning: WARNING: SQL-parsed foreign key constraint '('dataset_id', 'asset', 'id')' could not be located in PRAGMA foreign_keys for table asset_alias_asset
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/alembic/operations/batch.py", line 690, in drop_constraint
const = self.named_constraints.pop(const.name)
KeyError: 'dataset_alias_dataset_alias_id_fkey'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warnings:

/usr/local/lib/python3.9/contextlib.py:126 SAWarning: WARNING: SQL-parsed foreign key constraint '('alias_id', 'asset_alias', 'id')' could not be located in PRAGMA foreign_keys for table asset_alias_asset
/usr/local/lib/python3.9/contextlib.py:126 SAWarning: WARNING: SQL-parsed foreign key constraint '('dataset_id', 'asset', 'id')' could not be located in PRAGMA foreign_keys for table asset_alias_asset
/usr/local/lib/python3.9/contextlib.py:126 SAWarning: WARNING: SQL-parsed foreign key constraint '('alias_id', 'asset_alias', 'id')' could not be located in PRAGMA foreign_keys for table asset_alias_asset_event
/usr/local/lib/python3.9/contextlib.py:126 SAWarning: WARNING: SQL-parsed foreign key constraint '('event_id', 'asset_event', 'id')' could not be located in PRAGMA foreign_keys for table asset_alias_asset_event

batch_op.create_foreign_key(
constraint_name="toar_dag_id_fkey",
referent_table="dag",
Expand Down Expand Up @@ -321,13 +314,6 @@ def upgrade():
unique=False,
)

batch_op.create_foreign_key(
constraint_name="adrq_asset_fkey",
referent_table="asset",
local_cols=["asset_id"],
remote_cols=["id"],
ondelete="CASCADE",
)
batch_op.create_foreign_key(
constraint_name="adrq_dag_fkey",
referent_table="dag",
Expand Down Expand Up @@ -565,7 +551,6 @@ def downgrade():
with op.batch_alter_table("task_outlet_dataset_reference", schema=None) as batch_op:
batch_op.alter_column("asset_id", new_column_name="dataset_id", type_=sa.Integer(), nullable=False)

batch_op.drop_constraint("toar_asset_fkey", type_="foreignkey")
batch_op.drop_constraint("toar_dag_id_fkey", type_="foreignkey")

_rename_index(
Expand Down Expand Up @@ -600,7 +585,6 @@ def downgrade():
with op.batch_alter_table("dataset_dag_run_queue", schema=None) as batch_op:
batch_op.alter_column("asset_id", new_column_name="dataset_id", type_=sa.Integer(), nullable=False)

batch_op.drop_constraint("adrq_asset_fkey", type_="foreignkey")
batch_op.drop_constraint("adrq_dag_fkey", type_="foreignkey")

_rename_pk_constraint(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

"""
create foreign key constraints for assets.

Revision ID: c4a1639f0f67
Revises: 05234396c6fc
Create Date: 2024-11-22 09:49:41.813016

"""

from __future__ import annotations

from alembic import op

# revision identifiers, used by Alembic.
revision = "c4a1639f0f67"
down_revision = "05234396c6fc"
branch_labels = None
depends_on = None
airflow_version = "3.0.0"


def upgrade():
"""Apply create foreign key constraints for assets."""
with op.batch_alter_table("asset_dag_run_queue", schema=None) as batch_op:
batch_op.create_foreign_key("adrq_asset_fkey", "asset", ["asset_id"], ["id"], ondelete="CASCADE")

with op.batch_alter_table("task_outlet_asset_reference", schema=None) as batch_op:
batch_op.create_foreign_key("toar_asset_fkey", "asset", ["asset_id"], ["id"], ondelete="CASCADE")


def downgrade():
"""Unapply create foreign key constraints for assets."""
with op.batch_alter_table("task_outlet_asset_reference", schema=None) as batch_op:
batch_op.drop_constraint("toar_asset_fkey", type_="foreignkey")

with op.batch_alter_table("asset_dag_run_queue", schema=None) as batch_op:
batch_op.drop_constraint("adrq_asset_fkey", type_="foreignkey")
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
Add UUID primary key to ``task_instance`` table.

Revision ID: d59cbbef95eb
Revises: 05234396c6fc
Revises: c4a1639f0f67
Create Date: 2024-10-21 22:39:12.394079
"""

Expand All @@ -33,7 +33,7 @@

# revision identifiers, used by Alembic.
revision = "d59cbbef95eb"
down_revision = "05234396c6fc"
down_revision = "c4a1639f0f67"
branch_labels = "None"
depends_on = None
airflow_version = "3.0.0"
Expand Down Expand Up @@ -167,6 +167,32 @@ def _get_type_id_column(dialect_name: str) -> sa.types.TypeEngine:
return sa.String(36)


def create_foreign_keys():
for fk in ti_fk_constraints:
if fk["table"] in ["task_instance_history", "task_map"]:
continue
with op.batch_alter_table(fk["table"]) as batch_op:
batch_op.create_foreign_key(
constraint_name=fk["fk"],
referent_table=ti_table,
local_cols=ti_fk_cols,
remote_cols=ti_fk_cols,
ondelete="CASCADE",
)
for fk in ti_fk_constraints:
if fk["table"] not in ["task_instance_history", "task_map"]:
continue
with op.batch_alter_table(fk["table"]) as batch_op:
batch_op.create_foreign_key(
constraint_name=fk["fk"],
referent_table=ti_table,
local_cols=ti_fk_cols,
remote_cols=ti_fk_cols,
ondelete="CASCADE",
onupdate="CASCADE",
)


def upgrade():
"""Add UUID primary key to task instance table."""
conn = op.get_bind()
Expand Down Expand Up @@ -232,15 +258,7 @@ def upgrade():
batch_op.create_primary_key("task_instance_pkey", ["id"])

# Create foreign key constraints
for fk in ti_fk_constraints:
with op.batch_alter_table(fk["table"]) as batch_op:
batch_op.create_foreign_key(
constraint_name=fk["fk"],
referent_table=ti_table,
local_cols=ti_fk_cols,
remote_cols=ti_fk_cols,
ondelete="CASCADE",
)
create_foreign_keys()


def downgrade():
Expand Down Expand Up @@ -270,12 +288,4 @@ def downgrade():
batch_op.create_primary_key("task_instance_pkey", ti_fk_cols)

# Re-add foreign key constraints
for fk in ti_fk_constraints:
with op.batch_alter_table(fk["table"]) as batch_op:
batch_op.create_foreign_key(
constraint_name=fk["fk"],
referent_table=ti_table,
local_cols=ti_fk_cols,
remote_cols=ti_fk_cols,
ondelete="CASCADE",
)
create_foreign_keys()
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,5 @@ def downgrade():

with op.batch_alter_table("xcom", schema=None) as batch_op:
batch_op.drop_column("value_old")

op.drop_table("_xcom_archive", if_exists=True)
1 change: 0 additions & 1 deletion airflow/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ def import_all_models():
import airflow.models.serialized_dag
import airflow.models.taskinstancehistory
import airflow.models.tasklog
import airflow.providers.fab.auth_manager.models


def __getattr__(name):
Expand Down
2 changes: 1 addition & 1 deletion airflow/models/asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class AssetAliasModel(Base):
),
"mysql",
),
default=str,
default="",
nullable=False,
)

Expand Down
2 changes: 1 addition & 1 deletion docs/apache-airflow/img/airflow_erd.sha256
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7748eec981f977cc97b852d1fe982aebe24ec2d090ae8493a65cea101f9d42a5
9f404e900b4d166d170d878fc00cad8e4397a4288167eebef1d77281a881e92d
2 changes: 1 addition & 1 deletion docs/apache-airflow/img/airflow_erd.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 3 additions & 1 deletion docs/apache-airflow/migrations-ref.rst
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ Here's the list of all the Database Migrations that are executed via when you ru
+-------------------------+------------------+-------------------+--------------------------------------------------------------+
| ``486ac7936b78`` | ``d59cbbef95eb`` | ``3.0.0`` | remove scheduler_lock column. |
+-------------------------+------------------+-------------------+--------------------------------------------------------------+
| ``d59cbbef95eb`` | ``05234396c6fc`` | ``3.0.0`` | Add UUID primary key to ``task_instance`` table. |
| ``d59cbbef95eb`` | ``c4a1639f0f67`` | ``3.0.0`` | Add UUID primary key to ``task_instance`` table. |
+-------------------------+------------------+-------------------+--------------------------------------------------------------+
| ``c4a1639f0f67`` | ``05234396c6fc`` | ``3.0.0`` | create foreign key constraints for assets. |
+-------------------------+------------------+-------------------+--------------------------------------------------------------+
| ``05234396c6fc`` | ``3a8972ecb8f9`` | ``3.0.0`` | Rename dataset as asset. |
+-------------------------+------------------+-------------------+--------------------------------------------------------------+
Expand Down
4 changes: 3 additions & 1 deletion scripts/in_container/run_generate_migration.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,7 @@

cd "${AIRFLOW_SOURCES}" || exit 1
cd "airflow" || exit 1
airflow db reset
airflow db reset -y
airflow db downgrade -n 2.10.3 -y
airflow db migrate -r heads
alembic revision --autogenerate -m "${@}"
2 changes: 2 additions & 0 deletions tests/utils/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ def test_database_schema_and_sqlalchemy_model_are_in_sync(self):
lambda t: (t[0] == "remove_table" and t[1].name == "sqlite_sequence"),
# fab version table
lambda t: (t[0] == "remove_table" and t[1].name == "alembic_version_fab"),
# Ignore _xcom_archive table
lambda t: (t[0] == "remove_table" and t[1].name == "_xcom_archive"),
]

for ignore in ignores:
Expand Down
5 changes: 5 additions & 0 deletions tests_common/test_utils/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ def initial_db_init():
from airflow.www.extensions.init_appbuilder import init_appbuilder
from airflow.www.extensions.init_auth_manager import get_auth_manager

from tests_common.test_utils.compat import AIRFLOW_V_3_0_PLUS

db.resetdb()
if AIRFLOW_V_3_0_PLUS:
db.downgrade(to_revision="5f2621c13b39")
db.upgradedb(to_revision="head")
db.bootstrap_dagbag()
# minimal app to add roles
flask_app = Flask(__name__)
Expand Down
Loading