potiuk opened a new pull request, #72718:
URL: https://github.com/apache/airflow/pull/72718

   The scheduler evaluates task dependencies in-process, and two of those deps 
read XCom:
   
   - `NotPreviouslySkippedDep` reads `skipmixin_key` for every parent that 
inherits from `SkipMixin` — any Branch or ShortCircuit operator, so this is a 
common path.
   - Both it and `TriggerRuleDep` read `past_depends_met` when 
`wait_for_past_depends_before_skipping` applies.
   
   Both went through `TaskInstance.xcom_pull` → `XComModel.deserialize_value`, 
which is `json.loads(value, cls=XComDecoder)`. `XComDecoder.object_hook` calls 
the serialization module with `full=True`, so a nested dict carrying 
`__classname__` gets imported and instantiated.
   
   The values these deps actually consume are primitives — a mapping of lists 
of task ids, and a boolean. Nothing here needs an object rebuilt, and the 
scheduler is the wrong process to rebuild one in: the [security 
model](https://airflow.apache.org/docs/apache-airflow/stable/security/security_model.html)
 reserves it for code the Deployment Manager installed.
   
   ### Change
   
   - `XComModel.deserialize_value_primitives` — plain `json.loads`, no 
`object_hook`. A `__classname__` envelope comes back as an inert `dict`.
   - `LazyXComPrimitivesSelectSequence` alongside `LazyXComSelectSequence`, for 
the lazy path.
   - `full: bool = True` threaded through `TaskInstance.xcom_pull`; 
`full=False` at the four dep call sites.
   
   All three of `xcom_pull`'s return paths are covered — the direct one and 
**both** lazy-sequence ones. The lazy path is what a mapped task instance 
takes, so covering only the direct return would have left the hole open exactly 
there.
   
   `NotPreviouslySkippedDep` also now checks the skipmixin payload is a mapping 
before testing membership. It reads whatever the parent task wrote, and a 
scalar there previously raised `TypeError` inside dependency evaluation instead 
of being ignored.
   
   Task code pulling another task's return value is unchanged and still 
receives the original Python object. Only the scheduler-side dep reads are 
narrowed.
   
   ### Tests
   
   Two regression tests, both verified to fail on unpatched sources and pass 
with the change:
   
   - a serde envelope stored under `skipmixin_key` is read as an inert mapping 
rather than instantiated;
   - a scalar under the same key is ignored rather than raising.
   
   Both write through `XComModel.set(..., serialize=False)` with a 
pre-serialized string, which is what the Execution API does with a 
caller-supplied value — `serialize_value` would have rejected the payload, so 
testing through that door would have proved nothing.
   
   Suites run: `test_not_previously_skipped_dep.py` (8), 
`test_trigger_rule_dep.py` + `models/test_xcom.py` (202), `test_taskinstance.py 
-k xcom` (19). All pass.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   
   https://claude.ai/code/session_015DKYM5PoVgKC5JqxLso8rn
   


-- 
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]

Reply via email to