potiuk commented on PR #67503:
URL: https://github.com/apache/airflow/pull/67503#issuecomment-4683271250

   @ashb — gentle nudge on this one. It's now rebased on `main` (the 
`test_log.py` conflict from the #66571 revert is resolved) and green.
   
   The open question from my last comment is still the decision point: 
`mask_secret` is public API and routinely receives `Any` values, and `add_mask` 
deliberately masks non-`str`/`dict` iterables 
(`set`/`tuple`/`frozenset`/generators) element-wise — but `MaskSecret.value: 
JsonValue` rejects exactly those at construction, so today they're masked 
locally yet silently never registered with the supervisor. I'd rather not gate 
a redaction control on mypy, since users aren't required to run it.
   
   Two ways forward, your call:
   
   1. **Warning-only** (this PR as-is) — surface the silent failure.
   2. **Coerce + warn** — `list()` the `set`/`tuple`/`frozenset` before 
building the message so they actually propagate and get masked supervisor-side, 
leaving the warning as a genuine last-resort guard. I think this is the 
stronger fix and I'm happy to push it.
   
   Which would you prefer?
   
   ---
   Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk before posting
   


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