SEZ9 commented on PR #11496:
URL: https://github.com/apache/seatunnel/pull/11496#issuecomment-5469326597
Thanks @DanielLeens — answering your latest review directly, since it hadn't
gotten a response yet.
**On the Assert-sink bug you found:** good catch. The `field_type = string`
on a column that is actually `bigint` in the `f7ff90511f2c` "Fix sink config
options" commit is a genuine mistake on my side — not yet fixed. I'll correct
the `field_type` in `jdbc_mysql_json_params.conf` to match the actual column
type in the next push.
**On the CI premise correction:** understood, and thanks for tracing that.
If the "Build" check has never completed because each push cancels the
in-flight run, then after the `field_type` fix I'll stop pushing and let one
full run go green (or fail with real signal) before iterating further.
**On the unchanged core files:** you note `ParameterSplitter.java` and
`ConfigBuilder.java` are byte-identical to what you've already traced across
several rounds. Since they're unchanged, I'd like to explicitly close the loop
on the earlier points against those two files so nothing is left ambiguous:
- ConfigBuilder: the unresolved-substitution/`${key}` placeholder concern
(values rebuilt via `ConfigFactory.parseMap` becoming literal strings the final
`.resolve()` can't substitute), the `Collectors.toMap` behavior on duplicate
`-i` keys and null values, and the dropped `System.setProperty` export of `-i`
variables.
- ParameterSplitter: the stricter brace/bracket/quote validation rejecting
previously-accepted opaque values, and the delimiter-adjacency quote heuristic.
Could you confirm whether your "architecturally sound and fully compatible"
assessment covers all of those specific points as resolved/accepted, or whether
any of them still need a code change from me? A short per-point yes/no would be
enough for me to know exactly what remains.
Lastly, your review comment appears to have been cut off mid-sentence in the
description of `f7ff90511f2c` — if there was more after that point (e.g. the
rest of the E2E infra trace), could you repost the remainder?
Summary of my remaining actions: (1) fix the Assert-sink `field_type`
mismatch, (2) let a Build run complete uninterrupted, (3) act on whatever you
confirm is still open from the list above.
<!-- streview-comment:675 -->
--
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]
