SEZ9 commented on PR #12443:
URL: https://github.com/apache/seatunnel/pull/12443#issuecomment-5923549356
Thanks for the rebase onto `dev` (4c4fd615d) in
971fb98587b1f5e673fd65912a8e66cc53b69f11 and for the detailed status,
@hutiefang76.
I haven't been able to verify the changes from the diff yet, so a few points
still need confirmation before I can close them out:
- **F1 (decrypt path undoing the fix)** –
`ConfigBuilderTest#testInvalidPathKeysSurviveConfigShadeRoundTrip` running
`^t_nova_.*$` and reserved/escaped keys through
`ConfigShadeUtils.decryptConfig(ConfigBuilder.of(configMap))` sounds like the
right coverage. I'll confirm once I've re-reviewed the diff.
- **F2 (docs)** – thanks for the links to the key-semantics note in both
languages; I'll check the wording on re-review.
- **F5 (fix private to RestUtil)** – since the round-trip test now lives in
`ConfigBuilderTest`, it sounds like the handling moved into the shared
`ConfigBuilder.of(Map)` path. Could you confirm that in the PR description so
it's clear `RestUtil` no longer carries a one-off workaround?
- **F3 / F6 (`->` separator, embedded quotes, whitespace, `$`/`:`/`${...}`
tokens)** – your comment doesn't mention these cases specifically. Does the new
round-trip test (or `RestUtilTest`) include a key containing `->`, a key with
embedded `"`/`\`, and a key whose only offending token is `$` or `:`? If not,
please add them; if yes, just point me at the assertions.
- **F4 (REST submit path IT)** – is there an `RestApiIT` case that submits a
job over REST with a regex key, or do you consider the
`RestApiSubmitJobConfigShadeDecryptTest` run sufficient for that? Either is
fine, I'd just like the decision recorded.
- **F7 (title)** – please make sure the final title reflects the modules
actually changed.
As you noted, a fresh remote CI run on
971fb98587b1f5e673fd65912a8e66cc53b69f11 is still needed. Once that's in and
the points above are answered, I'll take another look.
<!-- streview-comment:1436 -->
--
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]
