SEZ9 commented on PR #12443: URL: https://github.com/apache/seatunnel/pull/12443#issuecomment-5987168422
Thanks @hutiefang76 for the detailed walkthrough against 971fb98587b1. I haven't yet verified these points against the current diff, so I'm not closing them out on the comment alone. Before I do a final pass, a few things would help: - **F1 / F5 / F7**: My earlier review recorded seatunnel-engine-server as the only changed module, while you now describe seatunnel-api and seatunnel-core-starter as the changed production modules. Could you confirm the current changed-file list so I can reconcile that and settle the `[Fix][Core]` vs `[Fix][Zeta]` question? Please also state explicitly in the PR description which test covers the decrypt-path issue (F1), so a future reader can connect the two. - **F3 / F6 / F4**: I'll check the tests you named (`RestUtilTest#buildConfigShouldKeepValidPathExpressions`, `buildConfigShouldPreserveEscapedAndReservedKeysThroughConfigShade`, `ConfigBuilderTest#testInvalidPathKeysSurviveConfigShadeRoundTrip`, `RestApiIT#testSubmitJobWithJsonFormat`, `RestApiSubmitJobConfigShadeDecryptTest#testSubmitJobWithRegexFieldInJson`) against the diff. Treating the existing quote/whitespace normalization of successfully parsed keys as pre-existing and out of scope is reasonable provided the description and the rest-api-v2 note say so. - **F2**: I'll confirm the key-semantics note in both docs/en and docs/zh rest-api-v2.md when reviewing the diff. Since the tests weren't re-run in this pass, please trigger a fresh remote CI run on the current head and ping me once it has a result. I'll then re-review the diff and the CI outcome together. <!-- streview-comment:1524 --> -- 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]
