kaxil commented on code in PR #71575:
URL: https://github.com/apache/airflow/pull/71575#discussion_r4128198297
##########
providers/common/ai/src/airflow/providers/common/ai/durable/fingerprint.py:
##########
@@ -98,19 +123,107 @@ def _strip_volatile(messages_dump: list[dict[str, Any]])
-> list[dict[str, Any]]
return stripped
+def _is_dataclass_instance(value: Any) -> bool:
+ return dataclasses.is_dataclass(value) and not isinstance(value, type)
+
+
+def _refuse_iterators(value: Any) -> None:
+ """
+ Raise ``TypeError`` if ``value`` holds an iterator anywhere inside it.
+
+ Rendering an iterator consumes it, and pydantic validates an
``Iterable[T]``
+ tool parameter lazily into a ``ValidatorIterator``. Tool arguments are
+ fingerprinted before the tool runs, so rendering them would drain the
tool's
+ own input: the tool would see an empty sequence, and that wrong result
would be
+ cached under the fingerprint of the full one. Refusing degrades the step
to the
+ not-cached path instead.
+ """
+ if value is None or isinstance(value, (str, bytes, bytearray, bool, int,
float)):
+ return
+ if isinstance(value, Iterator):
+ raise TypeError(f"cannot fingerprint {type(value).__name__} without
consuming it")
+ if isinstance(value, Mapping):
+ children: Iterable[Any] = value.values()
+ elif isinstance(value, (list, tuple, set, frozenset)):
+ children = value
+ elif isinstance(value, BaseModel):
+ children = [getattr(value, name, None) for name in
type(value).model_fields]
+ elif _is_dataclass_instance(value):
+ children = [getattr(value, field.name, None) for field in
dataclasses.fields(value)]
+ else:
+ return
+ for child in children:
+ _refuse_iterators(child)
+
+
+def _order_sets(value: Any, rendered: Any) -> Any:
+ """
+ Return ``rendered`` with every list that pydantic rendered from a set
sorted.
+
+ ``rendered`` is pydantic's JSON rendering of ``value`` and is otherwise
kept as
+ is, so pydantic stays the only renderer: bytes, dates, dict keys, ``NaN``
and
+ custom serializers come out exactly as a json-mode dump renders them. The
one
+ thing pydantic cannot do stably is order a set. It lists the members in
+ iteration order, which for strings follows the interpreter's hash seed, and
+ every task attempt is a fresh process, so a ``set[str]`` would hash
differently
+ on each attempt and never replay. ``value`` is walked alongside
``rendered``
+ only to find those lists; a branch whose rendering does not line up with
the
+ object, such as one with a custom serializer, is left exactly as rendered.
+ """
+ if isinstance(value, (set, frozenset)):
+ if not isinstance(rendered, list) or len(rendered) != len(value):
+ return rendered
+ members = [_order_sets(member, item) for member, item in zip(value,
rendered)]
+ return sorted(members, key=lambda member: json.dumps(member,
sort_keys=True))
+ if isinstance(value, Mapping):
+ if not isinstance(rendered, dict):
+ return rendered
+ if len(rendered) != len(value):
+ # Distinct keys that render alike, such as 1 and "1": the digest
could no
+ # longer tell those payloads apart, so refuse rather than hash
either one.
+ raise TypeError("dict keys collide once rendered as JSON")
Review Comment:
The same positional pairing can also replay a stale step, which is worse
than the spurious `None` above. A serializer that reorders keys without
dropping any keeps the lengths equal, so the zip at 185-188 pairs each value
with a neighbour's rendering. With `@field_serializer("filters")` returning
`dict(sorted(v.items()))` on `Query(filters={"status": {3, 7}, "order":
["created", "id"]})`, the set is paired with the rendered `order` list and
sorts it, so `order=["created", "id"]` and `order=["id", "created"]` get the
same non-`None` `fingerprint_tool_call` digest on every hash seed I tried,
where main returns `None` for both. So a length check alone won't cover it.
Comparing `list(to_jsonable_python(dict.fromkeys(value), bytes_mode="base64"))`
against the rendered keys, raising when it is shorter than `value` and
returning `rendered` when the keys differ, gave distinct digests for the two
orders, still refused `{1: "a", "1": "b"}`, still sorted a plain set, and
rendered the dropped
-`authorization` case unchanged.
--
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]
