ephraimbuddy commented on PR #69864: URL: https://github.com/apache/airflow/pull/69864#issuecomment-5623811388
> One note that doesn't fit inline: the description says the diff engine "is fully exercised by unit tests." The precedence issue I flagged in `_canonicalize_payload_v1` got through because the mapped-task default tests only shadow client defaults via `partial_kwargs`. No case sets a partial-only field at the task's _top level_, so that resolution path has no coverage at all. The one-line fix I tried passed all 186 existing tests, which is how I noticed the gap rather than the bug. > > Probably worth a parametrized case that sets `retries`/`pool` at the top level with a conflicting `client_defaults.tasks` value, and softening the claim in the description. Resolved alongside other reviews -- 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]
