23tae commented on code in PR #67900:
URL: https://github.com/apache/airflow/pull/67900#discussion_r3349369321
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/backfills.py:
##########
@@ -269,6 +284,14 @@ def create_backfill(
status_code=status.HTTP_400_BAD_REQUEST,
detail=str(e),
)
+ except OperationalError as e:
+ if "database is locked" in str(e):
+ raise HTTPException(
+ status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
+ detail="Backfill creation failed because the database is
locked. "
+ "This is a transient error — please retry the request.",
+ )
+ raise
Review Comment:
This looks duplicated. Since the first `except OperationalError` already
catches this exception type, the later handler appears unreachable. Could we
consolidate this into a single handler?
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_backfills.py:
##########
@@ -352,6 +353,34 @@ def test_create_backfill_with_depends_on_past(
== "Dag has tasks for which depends_on_past=True. You must
set reprocess behavior to reprocess completed or reprocess failed."
)
+ def test_create_backfill_database_locked(self, session, dag_maker,
test_client):
+ """Regression test for issue #66726: SQLite 'database is locked'
returns 503."""
+ with dag_maker(session=session, dag_id="TEST_DAG_1", schedule="0 * * *
*") as dag:
+ EmptyOperator(task_id="mytask")
+ session.scalars(select(DagModel)).all()
+ session.commit()
+
+ from_date = pendulum.parse("2024-01-01")
+ to_date = pendulum.parse("2024-02-01")
+
+ data = {
+ "dag_id": dag.dag_id,
+ "from_date": to_iso(from_date),
+ "to_date": to_iso(to_date),
+ "max_active_runs": 5,
+ "run_backwards": False,
+ "dag_run_conf": {},
+ }
+
+ with mock.patch(
+
"airflow.api_fastapi.core_api.routes.public.backfills._create_backfill",
+ side_effect=OperationalError("statement", "params", "database is
locked"),
Review Comment:
I ran this test locally and it fails — the `OperationalError` propagates
unhandled instead of returning 503.
The mock passes `"database is locked"` as a plain string for `e.orig`, but
the implementation checks `e.orig.args[0]`. Since strings don't have an `args`
attribute, the condition is never met and the exception is re-raised as-is.
To fix this, either:
1. Use a real exception object in the mock:
`sqlite3.OperationalError("database is locked")`
2. Or simplify the implementation to check `"database is locked" in str(e)`
instead
--
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]
