Eason09053360 commented on code in PR #73753:
URL: https://github.com/apache/airflow/pull/73753#discussion_r4115491336
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_state_store.py:
##########
@@ -267,7 +267,12 @@ def patch_task_state_store(
detail=f"Task state store key {key!r} not found",
)
- _get_db_backend().set(scope, key, json.dumps(body.value),
expires_at=existing.expires_at, session=session)
+ try:
+ _get_db_backend().set(
+ scope, key, json.dumps(body.value),
expires_at=existing.expires_at, session=session
+ )
+ except ValueError as e:
Review Comment:
A missing DagRun never reaches this: `_validate_scope` already returns 404
for it (`TestUnknownTaskInstance::test_returns_404[run_id-patch]`), so this
only fires if the run is deleted mid-request?
I think fine to keep for parity with PUT, but could the PR description say
that instead of the 500?
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_task_state_store.py:
##########
@@ -394,6 +394,21 @@ def test_patch_stores_json_encoded_value(self,
test_client, value, expected_db):
self._session.refresh(row)
assert row.value == expected_db
+ def test_patch_task_state_store_domain_error_returns_404(self,
test_client):
+ """Domain-level ValueError raised during PATCH translates to HTTP
404."""
+ _create_task_state_store_row(self._session, "job_id", "initial",
self.dag_run)
+ self._session.commit()
+
+ with patch(
+
"airflow.api_fastapi.core_api.routes.public.task_state_store._get_db_backend"
+ ) as mock_backend:
+ mock_backend.return_value.set.side_effect = ValueError(
+ f"No DagRun found for dag_id={DAG_ID!r} run_id={RUN_ID!r}"
+ )
Review Comment:
A bare mock accepts any arguments, so a typo like `expire_at=` in the route
would still pass here. Patching the real method with `autospec=True` checks the
call against `set()`'s signature:
```suggestion
with patch(
"airflow.state.metastore.MetastoreBackend.set",
autospec=True,
side_effect=ValueError(f"No DagRun found for dag_id={DAG_ID!r}
run_id={RUN_ID!r}"),
):
```
--
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]
