SEZ9 commented on PR #12443:
URL: https://github.com/apache/seatunnel/pull/12443#issuecomment-6392313832
@hutiefang76 thanks for the location-by-location breakdown of the seven
`parseMap` call sites on `509fddabe` (`ReadonlyConfig.toConfig`, the five
`ConfigBuilder` sites and `ConfigShadeUtils.processConfig`), each wrapping its
map with `quoteInvalidPathKeys` — that is exactly what I was after.
If that holds up in the diff, it addresses my two main concerns: the quoting
being undone by the second parse after shade processing on the REST submit
path, and the fix living only in `RestUtil` rather than the shared Map->Config
path. I'll confirm against the diff on this head before signing off.
A few of my earlier points I can't yet tie to the current head from the
thread; could you point me to where they stand?
- Keys that parse successfully: are `->` still split into nested paths and
embedded quotes still stripped, so JSON keys are not preserved literally? If
so, is that intended, and is it documented?
- Tests: is there coverage for a regex key through the shade/decrypt path,
an escape round-trip for `"`/`\` in keys, a top-level key, the `->` separator,
and a key whose only offending token is `$`/`:`/`${...}`?
- Docs: the REST API v2 docs (en and zh) should describe the new key
semantics for REST JSON bodies.
Once I've checked the diff and those are covered, I'll do my final pass.
Thanks again!
<!-- streview-comment:2007 -->
--
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]
