DanielLeens commented on PR #10252: URL: https://github.com/apache/seatunnel/pull/10252#issuecomment-5216070036
+1 to @SEZ9's review — Issue 1 in both of our reviews is the same root cause (`junit-jupiter-api` missing `<scope>test</scope>` in `connector-jdbc/pom.xml`, leaking into the runtime dependency surface), independently confirmed, so that stays the primary blocker alongside my own Issue 3 (real merge conflicts against current `dev`, `mergeable=CONFLICTING`/`mergeStateStatus=DIRTY` as of my last check). Your Issues 2-5 (the static `seatunnel.engine.port` system-property mutation, the leftover `test_catalog_table` + skipped `catalog.close()` on assertion failure, the inert `@Testcontainers` annotation, and the `JdbcDialect`-borrowed logger) are all things I didn't check in my own pass and look correct on the description — the `test_catalog_table` leak in particular is a good catch given the shared-container design I flagged as a lower-severity risk in my own review. I'll fold these in as additional non-blocking recommended fixes alongside my Issue 4 (the disabled `PostgresDialect.parse()` quoting test needing a tracking issue) once the author is back with an update — none of them change my Issue 1/2/3 blockers or the overall "Ready to merge after fixes" conclusion. -- 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]
