[dengn]

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

Closes #30060
Closes #30061
Closes #30062
Closes #30063

What this PR does / why we need it:

Problem and behavior

Lazy engine construction previously counted as a successful connection; TLS settings, failed-state cleanup, password diagnostics and async deadlines were incomplete.

Require an authenticated probe before reporting success, dispose/clear failed engines, preserve connection timeout overrides and sync socket/charset options, and apply the AsyncClient.execute deadline through pool acquisition, execution, fetch and transaction completion. Required synchronous TLS uses a verifying SSLContext and rejects peers without TLS before authentication, including old PyMySQL. Password prompts use mysql -p; connection errors are sanitized before logging/wrapping and suppress the original display chain. Keep the exact SQLAlchemy 1.4 aiomysql pre-ping compatibility guard.

The escaped-password follow-up handles both raw and repr/JSON-escaped forms, including backslashes, quotes, control characters and printable Unicode. Longer escaped forms are removed first to avoid partial replacements. Cancellation is not proof of server rollback; uncertain writes require reconciliation before retry. Independently managed Sessions retain their own deadline scope.

Connection policy: CONNECTION_POLICIES.md.

Validation (2026-10-18)

• Python 3.12 full offline suites: SQLAlchemy 1.4.54 1203, 2.0.55 1199, 2.1.5 1207, each plus 86 subtests, zero failures/skips.
• New public sync/async tests use real SQLAlchemy/PyMySQL/aiomysql connections to a local MySQL ERR peer: 6 failures and 2 passing plain-password controls on the previous head; 8 passed after correction. They inspect both the raised exception and the logged exception argument. Six additional wrapped-driver Unicode/quote-choice regressions pass.
• Existing TLS/deadline behavior remains covered by repository regression tests. Earlier dedicated TLS/native runs (source cd8d5ab) are historical; this follow-up does not claim a fresh TLS deployment certification. The latest review independently reported passing trusted/untrusted/hostname/no-TLS/disabled probes on PyMySQL 1.2.3 and 1.0.0, plus eight execution/commit cancellation/timeout probes. Those review results are not represented as new author runs.
• Black, isort, fatal Flake8 and diff checks pass.

CI for this commit: https://github.com/matrixorigin/matrixone/actions/runs/39125089824 (passed: six SDK offline cells and CI Required). Kernel/online jobs follow SDK scope. The latest reviewer decision is APPROVED. No manual queue or merge action is requested by this description update.

[MatrixAdventurer]

由LW授权的ChatGPT Business智能体提交。

P2: the pre-ping workaround still misses the supported SQLAlchemy 2.0.43 combination

Verified against current PR head e197b1d997200c09e43fb5b97bf1a493b9ca04db, specifically async_client.py lines 903–907. Related: #30242.

The version.startswith('1.4.') guard fixes that family but does not cover SQLAlchemy 2.0.43, which is still admitted by the package metadata. In 2.0.43, the PyMySQL dialect sees PyMySQL 1.2.3's default reconnect=False and calls the adapted connection's ping() without an argument. Its aiomysql adapter still defines ping(self, reconnect), so it fails before the underlying driver ping executes:

TypeError: AsyncAdapt_aiomysql_connection.ping() missing 1 required positional argument: 'reconnect'
underlying driver ping calls: 0

New isolated Python 3.12.14 diagnostic results, keeping PyMySQL 1.2.3 and aiomysql 0.3.2 fixed:

SQLAlchemy	Frozen PR factory's pre-ping boundary
1.4.54	Success; underlying ping called once with reconnect=False
2.0.43	TypeError above; underlying ping never called
2.0.54	Success; underlying ping called once with reconnect=False

The same 2.0.43 failure reproduces against main 17a6200f59e764729e5544908fffd5480ae88279. Directly calling that version's real adapter with False succeeds as a control.

Evidence scope: the diagnostic executes the AST-extracted, unchanged AsyncClient._create_async_engine method from each frozen source, then the real SQLAlchemy dialect/aiomysql adapter inside greenlet_spawn, with a recording async raw-ping stub. It does not import/run the entire SDK, perform actual pool checkout, or connect to a MatrixOne server. This proves the argument-binding failure, not an end-to-end integration pass. Expected TypeError is captured for reporting; the diagnostic's exit 0 is not a green regression test.

Please cover the affected adapter contract rather than only the 1.4 version prefix, or explicitly constrain/document unsupported dependency combinations, and include the reported 2.0.43 tuple in a connection-reuse/pre-ping regression. Testing only the latest 2.0 patch masks this gap. I have not inferred that every 2.x version is affected.

An independent read-only review confirmed this bounded finding and its evidence limits. No changes to your branch, no duplicate fix PR, and no whole-PR approval or request-changes review were submitted.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · done

Commit: b2577c9db696be757799ee5a72785db869cca96b

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · review

Commit: b2577c9db696be757799ee5a72785db869cca96b

Assessment: request_changes

Reviewed the complete eight-file delta, callers, lifecycle ownership, tests, documentation and supplied comments. Password redaction remains incomplete in both connection APIs.

Model: gpt-6-astra / xhigh

Session: 01a14d34-4e51-79a3-a62d-6c9c16279dc8 · slot 0 · generation 0

• P2 clients/python/matrixone/_connection_options.py:21: A connection error echoes a password containing a backslash. Driver exception formatting escapes the backslash before redact_error() performs its literal replacement.
  Impact: Client.connect() and AsyncClient.connect() expose the recoverable password in the raised ConnectionError and logged error argument. The PR's redaction contract remains unmet. Sanitize exception content before formatting or handle escaped representations, with regressions covering both APIs.
  Evidence: test_review_redaction.py reproduces this through public APIs and real SQLAlchemy/PyMySQL/aiomysql connections to a local MySQL ERR peer. SQLAlchemy 2.0.43 and 2.0.55 each produced two failing backslash-password cases and two passing plain-password controls. Retained redaction.log and redaction-current.log match exit-1 receipts checks/1792308744456445756-3.json and checks/1792309071444409192-3.json. This is an incomplete fix, not a claim that exposure originated in this PR.

Validation:

• Preflight passed; 49 focused repository offline tests passed.

• Five real-driver TLS probes passed with PyMySQL 1.2.3 and again with minimum-supported PyMySQL 1.0.0, covering trusted TLS, hostname mismatch, untrusted CA, absent TLS capability and disabled TLS.

• Eight phase-controlled async probes passed, covering timeout and cancellation during execution and commit, string and SQLAlchemy statements, pool cleanup and subsequent query recovery.

• Redaction regressions failed reproducibly on SQLAlchemy 2.0.43 and 2.0.55.

• Commands, source fingerprints, exits, logs and JUnit counts were reconciled in /home/xupeng/.local/state/mo-bot/jobs/a327ae54ddc5005d184fdd5d/attempt-1/validation-summary.json. Closing audit receipt checks/1792309224076467928-3.json records success; source remained unchanged.

Limitations:

• Transport probes used real SDK entry points and drivers against local protocol peers, not a MatrixOne deployment. They do not establish server rollback or durable commit outcomes.

• The supplied snapshot reports six successful offline CI cells; raw CI/author artifacts were unavailable and online CI was skipped. Standalone linked-issue bodies were absent.

• The reported SQLAlchemy 2.0.43/aiomysql pre-ping signature failure reproduces with both base and head factories. It remains a preexisting compatibility problem, excluded from introduced findings.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · done

Commit: faa4e4065e7ac0d6e7ba44e048ab82a063196cf3

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · review

Commit: faa4e4065e7ac0d6e7ba44e048ab82a063196cf3

Assessment: approve

Reviewed the complete nine-file delta, callers, lifecycle ownership, documentation and supplied comments. The earlier password-redaction finding is resolved. No actionable introduced defect remains within the reviewed contracts.

Model: gpt-6-astra / xhigh

Session: 01a14d34-4e51-79a3-a62d-6c9c16279dc8 · slot 0 · generation 0

Validation:

• Current-head redaction tests: 14 passed, zero failures or skips, including real sync/async driver connections and logged-exception checks. redaction.log/redaction.xml match receipt checks/1792325818039992624-3.json.

• Independently reconciled retained commands, source fingerprints, artifact hashes, logs and results for ten TLS probes and eight execution/commit interruption probes. Relevant implementations are unchanged; reuse-audit.log matches checks/1792325814605917358-4.json.

• Frozen exact-head evidence records six successful SDK offline CI cells. Preflight and final evidence audit passed; source remained unchanged.

• Complete scope and evidence mapping: /home/xupeng/.local/state/mo-bot/jobs/21d2cf5494c6c3e91096ae70/attempt-1/review-record.json. Final audit receipt: checks/1792325973323628768-4.json.

Limitations:

• Transport probes use real SDK APIs and drivers against local protocol peers, not a MatrixOne deployment. They establish client transport and cleanup behavior, not durable server rollback or commit outcomes.

• The SQLAlchemy 2.0.43 pre-ping incompatibility reproduces on the merge-base and remains a preexisting limitation.

• Online CI was skipped; historical native runs were not treated as current-head validation. Standalone linked-issue bodies and raw CI logs were not supplied.

[mergify]

Queued — the merge queue status continues in this comment ↓.

[mergify]

Merge Queue Status

• ✅ Entered queue — 2026-10-19 04:03 UTC · Rule: main · triggered by rule Automatic queue on approval for main
• ✅ Checks passed · in-place
• ✅ Merged — 2026-10-19 04:54 UTC · at 8ef2eaf4d923bdb4136dd81a50850e8eced0f368 · squash

This pull request spent 51 minutes 36 seconds in the queue, including 1 minute 54 seconds running CI.

Required conditions to merge

• #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • fix(python-sdk): enforce handshake, TLS, redaction and async deadlines #30074
• github-review-approved [🛡 GitHub branch protection]
  • fix(python-sdk): enforce handshake, TLS, redaction and async deadlines #30074
• any of [🛡 GitHub branch protection]:
  • check-success = CI Required
  • check-neutral = CI Required
  • check-skipped = CI Required