RehanAhmad25 commented on PR #72499:
URL: https://github.com/apache/airflow/pull/72499#issuecomment-5612942552

   @fat-catTW since you've been deep in this file across a few rounds now, 
wanted to get your take on a design question from @soupam05  above: moving the 
`conf` masking into `DAGRunResponse` itself via a `model_validator` (reading 
the resolving `dag` from `ValidationInfo.context`), instead of each route 
calling `_mask_password_conf`/`_build_masked_dag_run_responses` explicitly.
   
   The idea: any future endpoint returning `DAGRunResponse` gets masking 
automatically, and a missing `dag` in context would raise loudly instead of 
silently passing through unmasked, precedent already exists in 
`VariableResponse.redact_val()`.
   
   me and soupam wants input from other reviewers before settling on it, given 
it changes the current design. Would appreciate your view given you've caught 
the exact class of bug (forgetting to mask at a given endpoint) this would 
prevent going forward. No implementation yet either way, just gathering 
opinions first.


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