SEZ9 commented on PR #12443:
URL: https://github.com/apache/seatunnel/pull/12443#issuecomment-6137879511
Thanks for the detailed follow-up, @hutiefang76.
Since you mention the head is unchanged and no source or test change was
made, I can't yet verify the points below from the diff. Could you help me
close them out?
- **F1 / F5:** You describe the shared guard as applied at ConfigBuilder's
REST Map conversion, ConfigShadeUtils and ReadonlyConfig.toConfig(). Please
point me to where that lives in this PR so I can confirm it covers the decrypt
path. Keeping ConfigAdapter/file loading, MetadataProviderManager and
AssertSink out of scope with a note in the description is fine by me.
- **F2 / F3:** Please point me to the EN/ZH REST doc text that states only
path-parser-rejected keys become literal while parsable keys keep quote
removal, whitespace trimming and `->` nesting. If that text is already in this
PR, that addresses my concern.
- **F4 / F6:** Thanks for listing
`buildConfigShouldPreserveEscapedAndReservedKeysThroughConfigShade`,
`buildConfigShouldPreserveTopLevelInvalidPathKey` and
`buildConfigShouldKeepValidPathExpressions`. Two small asks: (a) does the
valid-path test include a `->` key and a key whose only offending token is `$`,
`:` or `${...}`? (b) Is there a REST-level IT that submits a regex-style key,
or are you relying on the unit tests only? Either answer is fine; I just want
it to be explicit before merge.
- **F7:** Still open — please retitle to `[Fix][Zeta]` per the commit
convention, since the changed module is the engine server.
On CI, thanks for looking into the failed jobs; please share what you find
and I'll take another pass once the above is answered.
<!-- streview-comment:1722 -->
--
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]
