DanielLeens commented on PR #11500:
URL: https://github.com/apache/seatunnel/pull/11500#issuecomment-6210065498
Thanks @SEZ9 for re-checking `f5fd0fdd06d0d2d8b5df367d735835bc1504e15f`. On
your open question about the shared e2e container config: it is unchanged on
this head.
`seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base/src/test/resources/seatunnel.yaml:26`
still has `dynamic-slot: true`, and the PR diff touches no file under
`seatunnel-e2e`. I filed this as a Low, non-blocking note in my last review
because the new `YamlSeaTunnelConfigParserTest` case already pins the default
itself and turns red if the flip is reverted. It would still be a nice widening
of coverage, so @zhangshenghang, either flipping that file or adding one
sentence to the PR description saying it is left on dynamic slots on purpose
would answer SEZ9's question.
On the two items you mentioned as merge-gate problems, they remain open on
this head, and the branch is currently `diverged` (ahead 14, behind 14):
- `engine-k8s-it` fails on both JDK lanes, and I suspect the new static
default against the 1-CPU pods in `seatunnel-statefulset.yaml` (2 slots per
worker, so about 4 in total, while the batch template needs 4). This is an
inference from the job names and the numbers; the `seatunnel-0` pod log would
confirm it.
- `docs/{en,zh}/introduction/concepts/incompatible-changes.md` conflict with
`dev`, so a sync is needed (please keep both sides).
I'll re-review as soon as a new commit lands.
--
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]
