SEZ9 commented on PR #12015: URL: https://github.com/apache/seatunnel/pull/12015#issuecomment-5852053274
Thanks for re-checking each point against `00acc23a582`. Here is where things stand from my side. **F1 (Compatibility)** — Agreed: with `ReadonlyConfig#toConfig()` still being `ConfigFactory.parseMap(confData)`, the finding as originally worded no longer applies. Two follow-ups on the places that did change: - `ConfigShadeUtils.processConfig` rebuilding the post-decrypt map via a JSON round trip: the reasoning that `configMap` is already the output of a JSON parse (so keys stay literal) sounds right. Since this runs for every job, could you add a small regression test on the shade path asserting that a config with a dotted key and a nested object both come out of `processConfig` unchanged, alongside the positive case for a regex key such as `^t_nova_.*$`? That would also cover the toConfig/shade half of F7. - `MultiTableFailureHelper.mergeOptions()` switching to a shallow top-level `putAll` merge is fine given the Javadoc now states it explicitly and current callers merge disjoint namespaces. No further ask here. **F2/F5 (declaration order)** — Your comment appears to cut off right after describing the three-pattern HOCON fixture in `testResolveMultiTablePrimaryKeysFromHoconFirstMatchWins`, so I can't see what the test asserts. Could you post the remainder? Specifically I'd like to confirm that (a) the exact-match pattern wins over the catch-all declared last, i.e. non-alphabetical declaration order is honored, and (b) the order guarantee holds on the option's underlying `Map<String, Object>` through the HOCON/JSON parse, `ReadonlyConfig`, and the shade round trip, not only in the test. If (b) holds, the `Jdbc.md` sentence is accurate and F5 can be closed together with F2. If the truncated part also covered the rest of the four open points, please re-post that as well so I can mark them off. Thanks! <!-- streview-comment:1346 --> -- 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]
