kaxil commented on code in PR #73005:
URL: https://github.com/apache/airflow/pull/73005#discussion_r4008787222
##########
airflow-core/src/airflow/models/xcom.py:
##########
@@ -245,6 +253,7 @@ def set(
dag_id=dag_id,
map_index=map_index,
dag_result=dag_result,
+ mapped_length=mapped_length,
Review Comment:
Yeah, fair. The old length probably isn't right anymore after a manual edit,
so forget carrying it forward. What I'd do instead is recompute from the new
value when it's a list or dict. Keeps it server-side too: you've already got
the loaded row and `patch_body.value` in `update_xcom_entry`, so
`XComUpdateBody` never needs a `mapped_length` field and the general purpose
API stays as it is.
The reason I'd still do something is that replace isn't the same event as
the retry purge, and it's a real button in the UI rather than a hypothetical
API caller. `XComModal.tsx`'s `onSave` sends just `{map_index, value}`, so any
edit from the XCom page nulls the length while the producer is still `SUCCESS`.
After that `get_task_map_length` reads NULL and expansion raises
`NotFullyPopulated`: an unexpanded downstream gets `UPSTREAM_FAILED`, an
already-expanded one keeps its TIs until `verify_integrity` marks every
`map_index >= 0` TI `REMOVED`. So you edit a bad mapping input, clear the
downstream, re-run, and nothing points back at the edit.
The same call nulls `dag_result` for the same reason, so it might be worth
fixing in the `set` signature rather than in this one caller.
Not going to hold the PR on it though, approving.
--
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]
