DanielLeens commented on PR #11169: URL: https://github.com/apache/seatunnel/pull/11169#issuecomment-5421782898
@SEZ9 Thanks for confirming and for turning this into a concrete punch list (F1-F8) -- that's exactly the scope needed to actually fix this rather than iterate on the framing again. Agreed on all six action items from your recap: 1. Replace the `e.getRawMessage().contains(DATABASE_NAME_REQUIRED_MESSAGE)` string match in `JdbcCatalogUtils.java` with a typed/structured signal, and wire the injected `database` key into a real `CatalogFactory` (starting with `SqlServerCatalogFactory`, since that's the scenario the PR description leads with) so the fallback and the injection actually connect to each other. 2. Fix the silent `Optional.empty()` degradation so it doesn't quietly disable schema-save-mode/auto-create, and make a blank `database` with a database-less URL fail fast with a clear config error instead of deferring to a runtime write failure. 3. Redact or drop the raw JDBC URL from the new fallback log line -- it can carry embedded credentials. 4. Restore a strict assertion in `JdbcStarRocksdbIT.java` rather than the current either/or, since StarRocks has no `CatalogFactory` and the loosened assertion doesn't prove anything about the new path. 5. Add the missing `ArgumentMatchers` static imports in `JdbcCatalogUtilsTest.java`. 6. Add a docs/changelog entry for the validation behavior change, and either land the Oracle thin-URL handling in this diff or drop that claim from the PR description if it's out of scope for this revision. No new commit has landed yet -- this stays a draft until the mechanism is proven end-to-end against a real `CatalogFactory`/`OptionRule` pipeline rather than the mocked `FactoryUtil` path the current tests use. I'll push a revision addressing F1-F7 (and resolve F4's Oracle scope question one way or the other) and flag it here once it's up so we can re-review together. -- 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]
