WXPNG commented on PR #12358: URL: https://github.com/apache/seatunnel/pull/12358#issuecomment-5714859327
Thanks for the thorough and detailed review — the analysis of the checkpoint/savepoint restore path was especially valuable. I've adopted **Option A** for both Issue 1 and Issue 2: the validation logic is now **only** invoked from the dry-run SPI methods (`validateConnectionForDryRun`), and has been completely removed from the production path (`restoreSource()` / `createSink()`). This addresses the core concern that the new connection/privilege check was silently entering the recovery path. Per-issue summary: 1. **Issue 1 (High)** — Fixed. Removed `validateMySqlPermissions()` from `restoreSource()`. The MySQL privilege check now runs only on the `--dry-run` path, never on checkpoint/savepoint reconstruction. 2. **Issue 2 (Medium)** — Fixed. Removed `validateElasticsearchConnection()` from `createSink()`. ES connectivity/index validation now runs only on the dry-run path. 3. **Issue 3 (Medium)** — Documented the MySQL 8 role-based grant limitation in both the Javadoc and the connector docs (en/zh). 4. **Issue 4 (Medium)** — Rewrote the four network-dependent MySQL tests to use `MockedStatic` for `MySqlConnectionUtils`/`MySqlConnection` (mirroring the Elasticsearch test pattern), removing the real DNS/network dependency. 5. **Issue 5 (Low)** — Added `--dry-run` validation notes to the MySQL-CDC and Elasticsearch connector docs (en/zh). 6. **Issue 6 (Low)** — Enabled GitHub Actions on my fork so CI can run against this branch. Regarding the API-submission-time validation you raised: I agree that belongs in a separate discussion/PR (with an explicit opt-in rather than a silent production-path change). I'll keep this PR scoped to the dry-run SPI only, as you recommended. Local verification: MySQL CDC tests 8/8, Elasticsearch tests 10/10 pass. PTAL, thanks again. -- 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]
