SEZ9 commented on PR #12810:
URL: https://github.com/apache/seatunnel/pull/12810#issuecomment-6432194903
@NganWave Thanks for the clear reply, and for being upfront about your time
— incremental commits are completely fine, no rush.
On the desensitization point: agreed, the broad masking work belongs in the
dedicated follow-up you describe. Tracing the whole path from
`ParameterSplitter` through `ConfigBuilder` and `ConfigUtil` down to
`ReadOnlyConfig` is the right shape for that, and it matches the plan from the
earlier PR you referenced. Please do link that PR back here when it's up.
One narrow ask I'd still like to keep in this PR: the two error messages
this change newly introduces in `PlaceholderUtils` (the conflicting-defaults
check and the unterminated-placeholder check) currently include the raw default
value / input slice. Since those are new surfaces created here rather than
pre-existing ones, simply dropping the raw text from those two messages (e.g.
report only the placeholder name and position) keeps this PR from adding to
what the follow-up has to clean up. Everything else on the masking side can
wait.
The remaining items from the earlier review are unchanged and can be picked
up one by one as you go:
- quoted `-i` values losing their string type hint before re-typing in
`ConfigBuilder`
- tightened `isStructured` in `ConfigValueUtils` demoting HOCON objects
without a top-level `:`/`=` to plain Strings
- system sink placeholders (`${table_name}` etc.) being subjected to the new
with/without-default consistency check
- quoted default values keeping their literal double quotes
- the `processPlaceholders` Javadoc declaring a different exception type
than the one actually thrown, plus missing Javadoc on the new public helpers
- `checkNotResolvedDefaultValue` swallowing non-`NotResolved` exceptions and
the "Structured default value" wording on the plain-string branch
- the EN doc bullet linking to the Chinese docs plus the broken markdown in
the preceding bullet
Ping me whenever you push and I'll re-review promptly.
<!-- streview-comment:2064 -->
--
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]
