hkc-8010 commented on code in PR #69403:
URL: https://github.com/apache/airflow/pull/69403#discussion_r4000217092
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -727,6 +728,15 @@ def get_previous_dagrun(self, state: str | None = None) ->
DagRun | None:
return response.dag_run
+ def update_dagrun_note(self, note: str | None) -> None:
+ """
+ Update the note for this task instance's DagRun.
+
+ A string sets or replaces the note and an empty string removes it.
``None`` is a
+ no-op, so an existing user-authored note is left untouched.
+ """
+ SUPERVISOR_COMMS.send(msg=UpdateDagRunNote(ti_id=self.id, note=note))
Review Comment:
Good call, done. The client now returns early on `None` so there is no
round-trip for a payload the server ignores. The server-side guard stays for
other lang SDKs that hit the endpoint directly.
##########
task-sdk/src/airflow/sdk/execution_time/schema/versions/v2026_10_30.py:
##########
@@ -15,6 +15,15 @@
# specific language governing permissions and limitations
# under the License.
+"""
+In-progress supervisor schema version.
+
+A brand-new message body needs no field-level migration instructions here: a
lang-SDK
+pinned to an older version simply never sends it, so there is nothing to strip
on the
+way down. Only *changes* to bodies that already existed at an earlier dated
version
+require a ``VersionChange`` entry below.
+"""
+
Review Comment:
Reworded to "Supervisor schema version 2026-10-30" so nothing says
in-progress after release.
I cannot drop the docstring entirely though.
`check_supervisor_schemas_versions` passes only when some file under
`versions/` is in the diff (`if version_files: return 0`), and this PR changes
the snapshot by adding `UpdateDagRunNote` to `ToSupervisor`, while
`schema/AGENTS.md` says a brand-new body needs no `VersionChange`. I ran the
hook both ways: with that file in the changed list it exits 0, without it the
run reaches the "schema has changed but no version file was updated" branch.
Happy to file a follow-up teaching the hook to allow net-new bodies without
a version-file touch, which would let this file drop out of the diff.
##########
airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_task_instances.py:
##########
@@ -4505,6 +4505,130 @@ def test_ti_patch_rendered_map_index_empty_string(self,
client, session, create_
assert response.status_code == 422
+class TestTIDagRunNoteUpdate:
+ def setup_method(self):
+ clear_db_runs()
+
+ def teardown_method(self):
+ clear_db_runs()
+
+ def test_create_dag_run_note(self, client, session, create_task_instance):
+ ti = create_task_instance(
+ task_id="test_create_dag_run_note",
+ state=State.RUNNING,
+ session=session,
+ )
+ ti_id = ti.id
+ session.commit()
+
+ response = client.patch(
+ f"/execution/task-instances/{ti_id}/dag-run-note",
+ json={"note": "Created from task runtime"},
+ )
+
+ assert response.status_code == 204
+ assert response.text == ""
+
+ session.expire_all()
+ dag_run = session.get(TaskInstance, ti_id).dag_run
+ assert dag_run.note == "Created from task runtime"
+ assert dag_run.dag_run_note.user_id is None
+
+ def test_update_dag_run_note_clears_user_id(self, client, session,
create_task_instance):
Review Comment:
Renamed to `test_runtime_update_of_user_note_becomes_unattributed` with a
docstring covering the intent. Reasoning for dropping the attribution is on the
route thread.
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py:
##########
@@ -908,6 +910,49 @@ def _raise_ti_not_in_live_table(task_instance_id: UUID,
session: SessionDep) ->
)
+@ti_id_router.patch(
+ "/{task_instance_id}/dag-run-note",
+ status_code=status.HTTP_204_NO_CONTENT,
+ responses=create_openapi_http_exception_doc(
+ [
+ (status.HTTP_404_NOT_FOUND, "Task Instance not found"),
+ (HTTP_422_UNPROCESSABLE_CONTENT, "Invalid payload for the DagRun
note update"),
+ ]
+ ),
+)
+def update_dag_run_note(
+ task_instance_id: UUID,
+ body: DagRunNoteUpdatePayload,
+ session: SessionDep,
+) -> None:
+ """
+ Update the note for the DagRun associated with this task instance.
+
+ An empty note removes the existing note, matching the public API. A null
note is a
+ no-op so runtime callers can leave a user-authored note untouched.
+ """
+ bind_contextvars(ti_id=str(task_instance_id))
+
+ dag_run = session.scalar(
+ select(DR)
+ .join(TI, and_(TI.dag_id == DR.dag_id, TI.run_id == DR.run_id))
+ .options(joinedload(DR.dag_run_note))
+ .where(TI.id == task_instance_id)
+ )
+ if dag_run is None:
+ raise HTTPException(
+ status_code=status.HTTP_404_NOT_FOUND,
+ detail={"reason": "not_found", "message": "Task Instance not
found"},
+ )
+
+ if body.note is None:
+ return
+
+ # Reuse the public API note logic so both editing paths stay consistent.
Runtime notes carry
+ # no acting user, so they are stored unattributed (user_id=None).
+ patch_dag_run_note(dag_run=dag_run, note=body.note, user_id=None)
Review Comment:
Kept `user_id=None` and added the log line you suggested.
The runtime write replaces the note content entirely, so carrying over the
previous author would credit them with text they did not write. An unattributed
note is the accurate record there. The route now logs dag_id, run_id and the
previous user_id whenever it replaces an attributed note, so the transition is
auditable.
--
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]
