RehanAhmad25 commented on PR #72499: URL: https://github.com/apache/airflow/pull/72499#issuecomment-5883448339
@fat-catTW You're right that `VariableResponse` isn't a fair comparison, it decides everything from its own fields, we need something external. One thing worth weighing though: the factory approach (`from_dag_run`/`build_masked_dag_run_response`) is more discoverable and type-checked, but it doesn't actually close the door on someone calling the plain `DAGRunResponse.model_validate(dag_run)` directly instead, that would still compile and silently return unmasked `conf`. The context-based validator's fail-loud behavior (raising if `dag` isn't in context) is what actually prevents that: there'd be no way to construct a valid instance without supplying it, even by accident. So maybe the two aren't mutually exclusive: keep the context-requiring `model_validator` on `DAGRunResponse` as the actual enforcement mechanism, but wrap it in a typed factory (`DAGRunResponse.from_dag_run(dag_run, dag)`) that's what every route actually calls, so you get the explicit, type-checked call site you want, and a stray bypass to plain `model_validate()` still fails instead of leaking. Curious if that addresses your concern or if I'm missing something about why that's worse than just the factory alone. CC: @soupam05. -- 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]
