SameerMesiah97 commented on code in PR #74890:
URL: https://github.com/apache/airflow/pull/74890#discussion_r4299679958
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dag_run.py:
##########
@@ -5099,6 +5099,29 @@ def test_bulk_update_note_only(self, test_client,
session):
assert dag_run.note == "bulk note"
assert dag_run.state == DAG1_RUN1_STATE
+ @pytest.mark.usefixtures("configure_git_connection_for_dag_bundle")
+ def test_bulk_update_listener_sees_note_when_note_and_state_both_updated(
+ self, test_client, listener_manager
+ ):
+ listener = ClassBasedListener()
+ listener_manager(listener)
+ response = test_client.patch(
+ self.ENDPOINT_URL,
+ json={
+ "actions": [
+ {
+ "action": "update",
+ "entities": [
+ {"dag_run_id": DAG1_RUN2_ID, "state": "success",
"note": "listener_note"}
+ ],
+ }
+ ]
+ },
+ )
+ assert response.status_code == 200
+ assert response.json()["update"]["success"] ==
[f"{DAG1_ID}.{DAG1_RUN2_ID}"]
+ assert listener.dag_run_note_at_listener == "listener_note"
Review Comment:
I think you need to cover the following scenarios as well:
1) The new note overriding the old note
2) New note being `None`
3) Falsy new note
Also cover both failing and successful DAG runs. Please see the below:
```
@pytest.mark.usefixtures("configure_git_connection_for_dag_bundle")
@pytest.mark.parametrize("state", [DagRunState.SUCCESS, DagRunState.FAILED])
@pytest.mark.parametrize(
("existing_note", "note_update", "expected_note"),
[
pytest.param(
None, {"note": "new note"}, "new note", id="new-note"
),
pytest.param(
"old note", {"note": "new note"}, "new note",
id="replace-stale-note"
),
pytest.param(
None, {"note": None}, None, id="none-without-existing-note"
),
pytest.param(
"old note", {"note": None}, "old note", id="none-preserves-note"
),
pytest.param(
"old note", {}, "old note", id="omitted-preserves-note"
),
pytest.param(
"old note", {"note": ""}, "", id="empty-string-replaces-note"
),
],
)
def test_bulk_update_listener_sees_expected_note(
self,
test_client,
listener_manager,
session,
state,
existing_note,
note_update,
expected_note,
):
dag_run = session.scalar(
select(DagRun).where(
DagRun.dag_id == DAG1_ID,
DagRun.run_id == DAG1_RUN2_ID,
)
)
assert dag_run is not None
dag_run.state = DagRunState.RUNNING
if existing_note is not None:
dag_run.note = (existing_note, "not_test")
session.commit()
listener = ClassBasedListener()
listener_manager(listener)
response = test_client.patch(
self.ENDPOINT_URL,
json={
"actions": [
{
"action": "update",
"entities": [
{
"dag_run_id": DAG1_RUN2_ID,
"state": state,
**note_update,
}
],
}
]
},
)
assert response.status_code == 200
assert response.json()["update"]["success"] == [
f"{DAG1_ID}.{DAG1_RUN2_ID}"
]
assert response.json()["update"]["errors"] == []
assert listener.state == [state]
assert listener.dag_run_note_at_listener == expected_note
session.expire_all()
dag_run = session.scalar(
select(DagRun).where(
DagRun.dag_id == DAG1_ID,
DagRun.run_id == DAG1_RUN2_ID,
)
)
assert dag_run.state == state
assert dag_run.note == expected_note
```
--
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]
