DanielLeens commented on PR #11186:
URL: https://github.com/apache/seatunnel/pull/11186#issuecomment-4968425404
Rechecked the same unchanged head `91d2d63a0277` after the CI / merge-gate
fact changed.
## Runtime path rechecked
```text
CLI
-> ClientCommandArgs.buildCommand() [ClientCommandArgs.java:147-152]
-> SeaTunnelConfValidateCommand.execute()
[SeaTunnelConfValidateCommand.java:92-180]
-> static config / DAG / option validation first
-> if dryRun == CONNECT
-> new DryRunConnectValidator(...).validate()
[DryRunConnectValidator.java:103-137]
source path
-> validateSource(...) [DryRunConnectValidator.java:140-200]
-> if factory implements SupportSourceDryRunValidation
-> inferSchemaForDryRun(...)
-> validateConnectionForDryRun(...)
-> otherwise
-> explicit schema from config => downstream schema checks still
meaningful
-> no explicit schema => mark as SKIPPED / unknown, no
placeholder-schema fake success
current JDBC source implementation
-> JdbcSourceFactory.inferSchemaForDryRun()
[JdbcSourceFactory.java:125-136]
-> JdbcCatalogUtils.getTables() [JdbcCatalogUtils.java:72-120]
-> findCatalog(...)
-> jdbcCatalog.open() [AbstractJdbcCatalog.java:171-173]
-> load real table metadata
sink path
-> validateSink(...) [DryRunConnectValidator.java:291-355]
-> if factory implements SupportSinkDryRunValidation
-> TableSinkFactoryContext.replacePlaceholderAndCreate(...)
-> validateConnectionForDryRun(...)
current JDBC sink implementation
-> JdbcSinkFactory.validateConnectionForDryRun()
[JdbcSinkFactory.java:270-339]
-> find catalog or plain JDBC connection
-> open connection
-> check target-table existence / field compatibility
failure propagation path
-> SimpleJdbcConnectionProvider.getOrEstablishConnection()
"No suitable driver found for " + jdbcConfig.getUrl()
[SimpleJdbcConnectionProvider.java:115-117]
or
AbstractJdbcCatalog.getConnection()
"Failed connecting to %s via JDBC."
[AbstractJdbcCatalog.java:150-156]
-> DryRunConnectValidator.wrap()
location(...) + " failed: " + e.getMessage()
[DryRunConnectValidator.java:464-472]
-> SeaTunnelConfValidateCommand.execute()
"Connectivity check failed: " + e.getMessage()
[SeaTunnelConfValidateCommand.java:175-180]
-> raw lower-layer message reaches CLI output
```
## Merge conclusion
### Conclusion: still needs fixes before merge
1. Blocking items
- The source-side blockers from my previous Daniel review still stand on
this unchanged head.
2. Current gate update
- The CI / merge-gate fact I referenced earlier has changed. The current
status is `CI is green on the current head`:
https://github.com/apache/seatunnel/actions/runs/28981801879/job/86002040236
So my technical conclusion is unchanged on merge readiness: this PR still
needs the earlier code-side fixes first.
--
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]
