DanielLeens commented on PR #11496:
URL: https://github.com/apache/seatunnel/pull/11496#issuecomment-5553497587
Thanks for consolidating the checklist for @NganWave, @SEZ9 — one correction
I want to flag before it goes into the rework list, since I don't want to send
them chasing something that's already resolved.
Your item 3 (`withFallback(cleanSourceConfig)` merging `-i` values as
top-level keys, risking secret exposure) — I traced this specific concern
end-to-end in my 2026-08-31 review (head `122e4a84c5df`) and it doesn't hold up
against the current source: `ConfigBuilder.java:301-306` strips every
placeholder key that wasn't already present in the *original* config back out
of `resolvedConfig` right after the `withFallback(...).resolve(...)` call,
specifically so a `${db_pwd}` substitution doesn't leave a root-level `db_pwd`
key behind. Nothing has changed in `ConfigBuilder.java` since then (no new
commits on the PR, still in Draft), so this should still be accurate. Unless
you've found a concrete case where that cleanup pass doesn't fire — e.g. a
placeholder key that collides with a real existing config path — I'd drop this
from the rework checklist rather than have @NganWave spend time re-scoping
something that's already handled.
The other three items in your list (duplicate `-i` key last-wins semantics,
the `System.setProperty` removal being an intentional and welcome drop of a
cross-job contamination risk, and the brace/quote tokenization change needing a
changelog note) match what's already in the combined blocker list from the
09-04 comment — no disagreement there.
@NganWave, no action needed on the secret-exposure point specifically; the
rest of the checklist (my 09-04 summary plus SEZ9's items 1, 2, and 4 above)
still stands for whenever you're ready to come out of Draft. Take the time you
need.
--
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]
