SEZ9 commented on PR #12443:
URL: https://github.com/apache/seatunnel/pull/12443#issuecomment-5842868466
Thanks @hutiefang76 for the update in f52673cf1, and for adding a regression
that reproduced the fallback-to-HOCON failure before the fix.
A few follow-ups before I can close out my earlier points:
1. **F1 / F5:** I'd like to confirm from the diff that every `Map`→`Config`
entry point (REST submit, the `ConfigShadeUtils.decryptConfig` step, and the
ConfigAdapter path) now goes through the same quoting guard. Could you list the
`ConfigFactory.parseMap` call sites the guard covers, and whether any callers
remain outside this PR?
2. **F3:** Keeping `->` splitting and the existing `env->job.mode` behavior
is a reasonable choice, but F3 was about successfully-parsed keys still being
silently rewritten (`->` splitting, quote stripping, whitespace trimming). If
that is intentional, please state it explicitly in the PR description so the
expected key semantics are clear.
3. **F4 / F6:** Could you point me to the new test cases covering nested
regex and escaped keys through the decrypt step, and note which reserved tokens
(e.g. `$`, `:`, `${...}`) the regex key exercises?
4. **F2 (docs):** Please add a short note to `rest-api-v2.md` in both
`docs/en` and `docs/zh` describing the resulting key semantics (reserved HOCON
characters taken literally, `->` still acting as the nesting separator).
5. **F7 (title):** Since the change now touches shared config code beyond a
single module, please make sure the PR title component reflects the actual
scope.
I'll take another look once CI on the new push has finished.
<!-- streview-comment:1327 -->
--
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]
