DanielLeens commented on PR #11399: URL: https://github.com/apache/seatunnel/pull/11399#issuecomment-5389990318
Thanks @SEZ9 for going back through this one with even more care — the additional test-hygiene points (the `MockJdbcConnection` never being closed, and the exact-SQL-string assertions being brittle) are good catches on top of Issue 1. To confirm we're aligned: my read matches yours. Issue 1 (`database.include.list` never actually being populated from `database-names`) is still the real blocker, since that's what keeps #8184 from being fixed for the standard config path — the guard in `TableDiscoveryUtils` is correct in isolation but never fires in production today. No new commit has landed on this head since my last review, so I'm not starting another full pass yet. Once a fix for Issue 1 lands (ideally with a non-mocked regression test per Issue 2, ideally reproducing the exact `database-names` config from the linked issue), I'll re-review from scratch and fold in your Issues 2-4 as well. Thanks again for staying on top of this. -- 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]
