SEZ9 commented on PR #11545:
URL: https://github.com/apache/seatunnel/pull/11545#issuecomment-5629300089
Thanks for the update on `fbd46ce70df3`. I understand this head is only a
`dev` sync on top of `c32b89fd4b1d` with no new changes to the PR's own logic,
so my earlier findings still apply as-is.
From the thread, 5 of the 8 findings were addressed in `504eb8b2979` and 3
are still open, but I don't see an explicit mapping of which IDs fall in which
bucket. Before I re-review, could you please:
1. List, per finding ID (PR11545-F1 through F8), whether it is fixed (with
the commit) or still open, so we're working from the same checklist rather than
a count.
2. For the open items, either push the fixes or explain why you'd prefer to
defer them. In particular I'd like a clear answer on:
- F1: whether the module flags now also live in the launch scripts (not
only the user-overridable `config/jvm_*_options` files), so an upgrade that
preserves an existing config directory doesn't silently lose them.
- F2: whether the Kerberos reflective reload now surfaces a visible
error/warning when the `--add-opens`/`--add-exports` flags are missing, instead
of failing silently.
- F4 / F7: how `upgrade_compatibility.yml` handles the old 2.3.13 release
on JDK 17 given it ships without the needed flags — is the scenario realistic
as run, or does it need an explicit flag injection for the old cluster?
- F5: whether there is any Java-version pre-check in the launch path so a
leftover JDK 8 fails with a clear message rather than `Unrecognized option`.
- F3 / F6 / F8: whether the blanket `ALL-UNNAMED` opens and the
workflow-level `JAVA_TOOL_OPTIONS` in `backend.yml` can be narrowed to only the
jobs/components that need them.
On sequencing: finishing the open items first and then doing a single rebase
on `dev` sounds fine to me. Please ping once that's done and I'll do a final
pass on the synced head.
<!-- streview-comment:964 -->
--
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]
