DanielCarter-stack commented on issue #10457: URL: https://github.com/apache/seatunnel/issues/10457#issuecomment-3857715148
<!-- code-pr-reviewer --> Thank you for proposing this improvement to stabilize the JDBC Schema Evolution E2E tests. This is a valuable contribution. Based on the codebase review: 1. **Network resilience**: Confirmed that `AbstractSchemaChangeBaseIT.java:159-174` uses `wget` without retry, while `DorisSchemaChangeIT.java:92` and `StarRocksSchemaChangeIT.java:120` use `curl`. Standardizing on `curl` or adding retry logic makes sense. 2. **Timeouts**: Most tests use 60s (e.g., `AbstractSchemaChangeBaseIT.java:295,310,324`), but `StarRocksSchemaChangeIT.java:171` uses 360s. Increasing to 180s is reasonable for slower CI runners. 3. **Health checks**: `SqlServerSchemaChangeIT.java:93-96` has `Wait.forLogMessage` but lacks `Wait.forListeningPort()` and JDBC connection validation. The sink containers in `AbstractSchemaChangeBaseIT.java:122-124` also lack unified readiness checks. 4. **Log consumers**: Already implemented in `SqlServerSchemaChangeIT.java:97-99`, so no changes needed there. **Clarification needed**: Could you provide a specific CI log link for the "Status 409: Container is not running" error mentioned? Also, should we add a unified `waitForSinkDbReady()` method to `AbstractSchemaChangeBaseIT`? Looking forward to your PR! -- 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]
