YAshhh29 commented on code in PR #71575: URL: https://github.com/apache/airflow/pull/71575#discussion_r4048355986
########## providers/common/ai/src/airflow/providers/common/ai/durable/fingerprint.py: ########## @@ -63,9 +65,13 @@ # Settings that control transport, not response content. Excluded from the # fingerprint: changing them should not invalidate a cached response, and some -# (``timeout`` can be an ``httpx.Timeout``) are not JSON-serializable, which -# would otherwise force the whole fingerprint to ``None`` and silently disable -# replay verification for every step. +# (``timeout`` can be an ``httpx.Timeout``) are not JSON-serializable. +# +# This frozenset is load-bearing. Model settings ride along with every request, +# so a single non-JSON member fingerprints every model step as ``None`` -- which +# now costs durable execution entirely for the run (nothing is cached, nothing +# is replayed), not merely the verification of a replay. Any non-JSON setting +# must be listed here or normalized before it reaches the fingerprint. Review Comment: Confirmed, and I'm glad you caught this one. It's a pre-existing bug, but my last commit turned it from "replays unverified" into "never cached at all", which is the worse of the two. I reproduced all three on 2.29.0 before touching anything: a `datetime` argument, a `Decimal` argument, and `ToolOrOutput(function_tools=[...])` in `tool_choice` each come back `None` today. The tool one bothers me most — an ordinary typed parameter isn't an edge case, and it fails silently. `to_jsonable_python` before `json.dumps`, as you suggested. I checked the three properties it leans on rather than taking them on trust: `PydanticSerializationError` subclasses `ValueError`, so the existing handler catches it unchanged; `extra_body=object()` still raises, so the unverifiable path is intact; and plain values round-trip byte-identically. That last one now has a test asserting `_digest` of a plain payload equals its pre-normalization sha256, so in-flight entries can't drift if pydantic changes later. `timeout` as an `httpx.Timeout` still fails under pydantic, so I've kept `_TRANSPORT_ONLY_SETTINGS` and just reworded the comment to draw the line at what pydantic can't serialize rather than what isn't JSON. -- 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]
