SEZ9 commented on PR #12443:
URL: https://github.com/apache/seatunnel/pull/12443#issuecomment-6331686895
Thanks for the update on `3d6c855f`, @hutiefang76.
Moving the guard into the shared `ReadonlyConfig` / `ConfigBuilder` map
conversions and the `ConfigShadeUtils` decrypt re-parse is the direction I was
hoping for on PR12443-F1 and PR12443-F5, and documenting `->` splitting and
parser normalization as intended behaviour is a reasonable way to address
PR12443-F2 and PR12443-F3. Keeping the metadata-provider follow-up out of this
PR is fine.
Before I can mark those resolved I need to verify them against the diff
rather than the description, so a few asks:
1. **F1 / F5** — please point me to the changed hunks in `ReadonlyConfig`,
the `ConfigBuilder` map conversions, and the `ConfigShadeUtils` re-parse so I
can confirm the guard is applied consistently.
2. **F2 / F3** — please link the EN/ZH doc changes; I want to confirm the
text is explicit that reserved HOCON characters are now literal while `->`
remains a path separator and that no byte-for-byte preservation is implied.
3. **Tests (F4 / F6)** — you mention REST and engine coverage; please point
me to the specific cases for a regex key (e.g. `^t_nova_.*$`) going through
`decryptConfig`, an escape round-trip with `"` and `\` in a key, a top-level
key, a key whose only offending token is `$`/`:`/`${...}`, and the `->`
separator behaviour. If any are missing, please add them; a RestApiIT case for
the actual submit path would be ideal.
4. **PR title (F7)** — now that the production changes live in
`seatunnel-api` and `seatunnel-core-starter`, please adjust the title prefix to
match the modules actually touched per the commit convention.
5. **CI** — as you noted, this still needs to pass before merge.
Once those are in I'll do a final pass.
<!-- streview-comment:1940 -->
--
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]
