DanielLeens commented on PR #10946:
URL: https://github.com/apache/seatunnel/pull/10946#issuecomment-4538715136
Thanks for the update. I re-reviewed the latest head from scratch. The
Kerberos negative-path blocker I raised earlier is fixed on the current
revision: the test now asserts only on newly appended broker logs instead of
the cumulative broker log.
# What this PR fixes
- User pain: the Kafka E2E suite, especially the Kerberos negative path, was
vulnerable to false greens because it asserted against cumulative logs and used
weaker readiness patterns.
- Fix approach: the latest head records brokerLogOffsetBeforeJob and only
inspects the new broker log segment for the current run.
- One-line summary: the Kerberos false-positive issue is fixed, but the
current Build still fails in another Kafka test in the same suite, so the
branch is not merge-ready yet.
# Runtime chain I checked
~~~text
Kerberos negative path
-> submit kafka_sink_with_not_kerberos.conf
-> wait until the job reaches RUNNING
-> read only kafkaContainer.getLogs().substring(brokerLogOffsetBeforeJob)
-> assert the authentication failure on this run-specific log segment
~~~
# Findings
- No new source-level blocker found in the Kerberos negative-path fix itself.
# CI note
- The current Build is still red on nzw921rx/seatunnel run 26398535223.
- The failing lane I traced is kafka-connector-it (8, ubuntu-latest),
specifically KafkaIT.testKafkaToKafkaExactlyOnceOnStreaming{TestContainer}[2]
timing out at KafkaIT.java:1686.
- KafkaKerberosIT itself passed in that run, so the remaining gate is
elsewhere in the same Kafka suite.
# Merge conclusion
### Conclusion: can merge after fixes
1. Blocking items
- Please get the remaining kafka-connector-it failure green first,
especially the exactly-once streaming case.
2. Suggested non-blocking follow-up
- The new run-scoped broker-log assertion pattern is a good stability
improvement and may be worth applying to similar negative-path tests elsewhere.
Overall, the Kerberos false-positive bug is fixed, but the suite is not
fully stable yet, so I would still wait for a green Kafka Build before merge.
--
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]
