SEZ9 commented on PR #12443:
URL: https://github.com/apache/seatunnel/pull/12443#issuecomment-6007872672
Thanks @hutiefang76 for the detailed follow-up against
`971fb98587b1f5e673fd65912a8e66cc53b69f11`.
I see the changed-file list now shows the guard in the shared
`ReadonlyConfig` / `ConfigMapPathUtils` / `ConfigBuilder` / `ConfigShadeUtils`
conversion code rather than `RestUtil.java`, plus the EN/ZH `rest-api-v2.md`
updates and `ConfigBuilderTest#testInvalidPathKeysSurviveConfigShadeRoundTrip`
for the decrypt path. That is the direction I was hoping for on F1, F2, F5 and
F7. Before I mark them resolved I'd like to verify against the diff itself, so
a few asks:
1. F1/F5 — could you point me to the hunk in `ConfigMapPathUtils` /
`ConfigBuilder` where the guard is applied, and the
`ConfigShadeUtils.decryptConfig` change that keeps it from re-parsing the map?
2. F2 — a short quote of the new `rest-api-v2.md` wording would help, in
particular what it says about the `->` separator versus reserved HOCON
characters.
3. F3/F6 — which test method covers a key containing `->`, a key whose only
reserved token is `$`/`:`/`${...}`, and a key with embedded `"`/`\` or
leading/trailing whitespace? Please also state the intended semantics for `->`
(literal vs. path separator) so it matches the docs.
4. F4 — which `RestApiIT` case submits a regex key such as `^t_nova_.*$`
through the actual REST submit path, and where is the top-level-key case
covered?
5. Could you share the result of the re-run Build workflow on the current
head?
Once I can confirm those in the diff, I'm happy to move forward.
<!-- streview-comment:1538 -->
--
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]
