jason810496 commented on code in PR #54767:
URL: https://github.com/apache/airflow/pull/54767#discussion_r2292546976
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py:
##########
@@ -217,7 +221,10 @@ def update_hitl_detail(
status.HTTP_409_CONFLICT,
]
),
- dependencies=[Depends(requires_access_dag(method="PUT",
access_entity=DagAccessEntity.HITL_DETAIL))],
+ dependencies=[
+ Depends(requires_access_dag(method="PUT",
access_entity=DagAccessEntity.HITL_DETAIL)),
+ Depends(action_logging()),
+ ],
Review Comment:
Do this PR need to wait until [Merge mapped and non-mapped HITL endpoints
into one #54723
](https://github.com/apache/airflow/pull/54723) get merged?
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_hitl.py:
##########
@@ -260,17 +262,45 @@ def expected_sample_hitl_detail_dict(sample_ti:
TaskInstance) -> dict[str, Any]:
}
[email protected](autouse=True)
+def cleanup_audit_log(session: Session) -> None:
+ session.query(Log).delete()
+ session.commit()
+
+
+def assert_audit_log(audit_log: Log, event: str) -> None:
+ assert audit_log.dag_id == DAG_ID
+ assert audit_log.task_id == TASK_ID
+ assert audit_log.run_id == "test"
+ assert audit_log.map_index is None
+ assert audit_log.try_number is None
+ assert audit_log.owner == "test"
+ assert audit_log.owner_display_name == "test"
+ assert audit_log.event == event
+ assert (
+ audit_log.extra
+ == '{"chosen_options": ["Approve"], "params_input": {"input_1": 2},
"method": "PATCH"}'
+ )
Review Comment:
Would it be better to pass `dict` as parameter instead of hardcode the
expected extra here?
No strong opinion, hardcode might also be enough as it _seems_ all test
tests use `sample_update_payload` fixture as API body.
--
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]
