SEZ9 commented on PR #12443: URL: https://github.com/apache/seatunnel/pull/12443#issuecomment-5852010413
Thanks for the detailed CI breakdown on `8ff338252`. Since this PR doesn't touch the checkpoint code or those tests, I'm fine treating the `BackpressureSlowSinkIT` / `CheckpointCoordinatorFailoverIT` failures in the JDK 8 ([108333703754](https://github.com/hutiefang76/seatunnel/actions/runs/36216457338/job/108333703754)) and JDK 11 ([108333703720](https://github.com/hutiefang76/seatunnel/actions/runs/36216457338/job/108333703720)) engine E2E jobs, plus the Windows PayPal and MinIO image-pull failures, as unrelated for the purposes of this review. No need to keep re-running the fork build for those. Good to hear `RestApiIT` passed 22/22 with the JSON regression. Could you point me at the new `RestApiIT` case? I'd like to confirm it goes through the actual REST submit path (`buildConfig` followed by `ConfigShadeUtils.decryptConfig()`) with a regex key like `^t_nova_.*$`. That's the crux of the remaining blocker: if `decryptConfig()` re-materialises the config as a Map and calls `ConfigFactory.parseMap` again, the quoting in `RestUtil` is undone before the job is actually submitted. Remaining asks, unchanged from the previous round: 1. **decryptConfig re-parse** – confirm/fix that the quoted key survives `ConfigShadeUtils.decryptConfig()` in `JobInfoService.submitJob/submitJobs` and `EncryptConfigService`, and cover it with a test that runs that path end to end. 2. **Literal key preservation** – "quote only on BadPath" still lets successfully parsed keys be rewritten (`->` split into nested paths, embedded quotes stripped, surrounding whitespace trimmed). Either quote unconditionally or document precisely which keys are preserved. 3. **Shared root cause** – the fix lives only in `RestUtil` while the problem is in `ConfigBuilder.of(Map)` / `ConfigFactory.parseMap`; please either move it there or explain why REST should diverge from the other Map→Config entry points. 4. **Unit tests** – add cases in `RestUtilTest` for `"`/`\` escape round-trips, a top-level offending key, the `->` separator, and keys whose only offending token is `$`, `:` or `${...}`; the current "dotted key" test doesn't hit the new quoting branch since `.` isn't a path separator in our fork. 5. **Docs** – update `docs/en` and `docs/zh` `rest-api-v2.md` to describe the new key semantics for JSON bodies. 6. **Title** – please change `[Fix][Core]` to `[Fix][Zeta]` since only `seatunnel-engine-server` changes. Once 1 and 4 are in I'll do another pass. <!-- streview-comment:1340 --> -- 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]
