kaxil commented on code in PR #44221:
URL: https://github.com/apache/airflow/pull/44221#discussion_r1854518660
##########
airflow/migrations/versions/0048_3_0_0_add_dag_versioning.py:
##########
@@ -131,6 +131,7 @@ def upgrade():
def downgrade():
"""Unapply add dag versioning."""
+ _delete_serdag_and_code()
Review Comment:
Does this fix any inconsistency? If not might be better as a separate PR
##########
airflow/migrations/versions/0050_3_0_0_remove_pickled_data_from_xcom_table.py:
##########
@@ -136,6 +136,7 @@ def upgrade():
# Drop the old `value_old` column
with op.batch_alter_table("xcom", schema=None) as batch_op:
batch_op.drop_column("value_old")
+ op.drop_table("_xcom_archive")
Review Comment:
I didn't drop it purposely to avoid loss of data, was planning to handle it
as part of `db cleanup`
##########
airflow/migrations/versions/0032_3_0_0_drop_execution_date_unique.py:
##########
@@ -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=UtcDateTime,
Review Comment:
I think we should use `airflow.migrations.db_types.TIMESTAMP` instead
https://github.com/apache/airflow/blob/5b2a96ee72a4bd7b7fbd4925c2f8c8468f047a64/airflow/migrations/db_types.py#L68
##########
airflow/migrations/versions/0041_3_0_0_rename_dataset_as_asset.py:
##########
@@ -565,7 +551,7 @@ 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_asset_fkey", type_="foreignkey")
Review Comment:
?
##########
airflow/migrations/versions/0041_3_0_0_rename_dataset_as_asset.py:
##########
@@ -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",
- )
Review Comment:
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'
```
##########
airflow/migrations/versions/0050_3_0_0_remove_pickled_data_from_xcom_table.py:
##########
@@ -101,9 +101,9 @@ def upgrade():
op.execute(
"""
ALTER TABLE xcom
- ALTER COLUMN value TYPE JSONB
+ ALTER COLUMN value TYPE JSON
USING CASE
- WHEN value IS NOT NULL THEN CAST(CONVERT_FROM(value, 'UTF8')
AS JSONB)
+ WHEN value IS NOT NULL THEN CAST(CONVERT_FROM(value, 'UTF8')
AS JSON)
Review Comment:
handled in https://github.com/apache/airflow/pull/44290
##########
airflow/migrations/versions/0041_3_0_0_rename_dataset_as_asset.py:
##########
@@ -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",
- )
Review Comment:
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
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
