JeremyXin commented on PR #11791:
URL: https://github.com/apache/seatunnel/pull/11791#issuecomment-5371623534
Reviewed the latest head eb566e5c8. The changes look good overall — found
one issue worth confirming before merge:
**Issue 1(MAJOR)**: Direct-module trigger runs with both RUN_ALL_CONTAINER
and RUN_ZETA_CONTAINER as 'false'
Location: .github/workflows/backend.yml — iceberg-connector-it (L1165) /
hbase-connector-it (L1194)
Description: The old all-connectors-it-8 was gated by api == 'true' ||
engine == 'true', so at least one of RUN_ALL_CONTAINER / RUN_ZETA_CONTAINER was
always 'true'. The new jobs add a third trigger — contains(it-modules,
'connector-iceberg-e2e') — while copying the env block verbatim. A PR that only
touches the Iceberg/HBase connector will fire the job with both flags as
'false', a combination these suites never ran with before. If the E2E framework
gates container startup on these flags, direct connector changes would run with
reduced coverage.
Suggestion: Verify how the E2E framework handles double-false. If containers
are skipped, consider deriving the flag from the full trigger condition (e.g.
RUN_ALL_CONTAINER: ${{ needs.changes.outputs.api == 'true' ||
contains(needs.changes.outputs.it-modules, 'connector-iceberg-e2e') }}). If
this is already a known-safe pattern shared by the other 12+ dedicated
connector jobs from #11291, a brief comment noting it is intentional would help.
--
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]
