ferruzzi commented on code in PR #66350:
URL: https://github.com/apache/airflow/pull/66350#discussion_r3994221518
##########
airflow-core/tests/unit/utils/test_db_cleanup.py:
##########
@@ -551,6 +668,83 @@ def
test_dag_version_cleanup_skips_versions_pinned_by_task_instance(self):
assert latest_id in remaining # kept by keep_last
assert orphan_id not in remaining # old and unreferenced -> pruned
+ def test_do_delete_skip_if_referenced_guards_against_race(self):
+ """_do_delete must not issue a DELETE that violates an ON DELETE
RESTRICT FK.
+
+ Simulates a race where a dag_version row passes the SELECT filter (no
TI
+ references it at archive-creation time) but a TI referencing it is
inserted
+ before the DELETE runs. The skip_if_referenced guard on the DELETE
itself
+ must leave the row in place instead of failing with IntegrityError.
+ """
+ from airflow.utils.db import reflect_tables
+
+ base_date = pendulum.DateTime(2020, 1, 1,
tzinfo=pendulum.timezone("UTC"))
+ bundle_name = f"race-test-{uuid4()}"
+ dag_id = f"race_dag_{uuid4()}"
+
+ with create_session() as session:
+ session.add(DagBundleModel(name=bundle_name))
+ session.flush()
+ session.add(DagModel(dag_id=dag_id, bundle_name=bundle_name))
+ session.flush()
+
+ dv = DagVersion(
+ dag_id=dag_id,
+ version_number=1,
+ bundle_name=bundle_name,
+ created_at=base_date,
+ last_updated=base_date,
+ )
+ session.add(dv)
+ session.flush()
+ dv_id = dv.id
+
+ # Manually create an archive table containing this dag_version row,
+ # simulating the CTAS step that ran before the TI was inserted.
+ # Use SQLAlchemy's Uuid codec to insert so the id encoding matches
how
+ # DagVersion.id was stored (hyphenated native uuid on Postgres,
.hex on
+ # MySQL/SQLite); a raw f-string would use the hyphenated form
everywhere
+ # and match nothing on MySQL/SQLite.
+ archive_name = f"{ARCHIVE_TABLE_PREFIX}dag_version__race_test"
+ stmt = text(
+ f"CREATE TABLE {archive_name} AS SELECT * FROM dag_version
WHERE id = :dv_id"
+ ).bindparams(bindparam("dv_id", value=dv_id, type_=Uuid()))
Review Comment:
Tiny nit, feel free to disagree and resolve this: I'd suggest
`type_=DagVersion.id.type` here. If that type ever changes, your way would
start failing and mine would pick that change up at the source. It may be
overly defensive though; I can't see ever really changing it away from UUID,
but who knows what the future brings...
--
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]
