feat(router): add SQL query support to both SDKs#1129
Conversation
3445b09 to
7860f91
Compare
PR Review: FAILWhat This DoesAdds Blast RadiusRouter SDK layer only: Python router export/method and TypeScript router export/method. The sidecar/core SQL endpoint already exists under Consumer VerificationBefore (base branch): After (PR branch): url = f"{self._resolve_sidecar_host()}/v0/sql"However the TypeScript method calls: await this.sidecarReadRequest('sql', { query }, [{ query }])
Test Results
Findings
PMXT Pipeline Check
Semver Impactminor -- adds a new Router SQL SDK method. RiskPython appears to target the right endpoint, but TypeScript is not safe to ship as-is because its public method will not reach the intended |
PR Updated ✅TypeScript SQL Fix:
Verification:
Ready for re-review! 🙏 |
PR Review: PASS (NOT VERIFIED)What This DoesAdds Blast RadiusPython SDK router/export surface and TypeScript SDK router/export surface. The core Consumer VerificationBefore (base branch): router.sql("SELECT 1 AS one")
# base branch: method is absent from sdks/python/pmxt/router.pyawait router.sql('SELECT 1 AS one')
// base branch: method is absent from sdks/typescript/pmxt/router.tsAfter (PR branch): POST /v0/sql {"query":"SELECT 1 AS one"}{
"error": "service_unavailable",
"message": "SQL query service is not available. Configure CLICKHOUSE_HTTP_URL or use the hosted PMXT Enterprise SQL endpoint."
}Python files compile, and the new SDK methods are present. Full SDK consumer verification against a live SQL backend was not possible in this environment. Test Results
FindingsNo blocking findings. PMXT Pipeline Check
Semver Impactminor -- adds new public SDK methods/types. RiskThe actual SQL query path could not be verified without a configured SQL backend. There are also no regression tests proving the Python/TypeScript SDK methods parse successful SQL responses, so this review is limited to static SDK inspection plus route reachability/service-unavailable evidence. |
|
Thank you for the PASS review . I have manually tested them : Python: from pmxt import Router, SqlResult, SqlMeta, SqlColumn
router = Router(pmxt_api_key='test')✅ Router created Types imported successfully Typescript import { Router } from 'pmxtjs';
const router = new Router({ pmxtApiKey: 'test' });✅ Router created Types ✅ |
c497de8 to
91ec8d9
Compare
- Add sql(query: string) method to Router class - Add SqlResult, SqlMeta, SqlColumn types - Export types from both SDKs Fixes pmxt-dev#1032
4028530 to
7a54c84
Compare
- Restore full __init__.py from upstream - Add SQL exports (SqlResult, SqlMeta, SqlColumn) Fixes CI for pmxt-dev#1032
- Add SqlResult, SqlMeta, SqlColumn imports - Add SQL exports to __all__ Fixes CI for pmxt-dev#1032
PR Review: FAILWhat This DoesIntends to expose router SQL query support through both SDKs and generated docs/references. Blast RadiusPython SDK package root/router, TypeScript SDK client/router, and generated API docs. This is SDK-publishing critical because it touches package files shipped to PyPI/npm. Consumer VerificationBefore (base branch): After (PR branch): Test Results
Findings
PMXT Pipeline Check
Semver Impactminor -- new SDK feature, but current artifact encoding is blocking. RiskVery high: publishing this head can ship unreadable SDK source files to npm/PyPI consumers. |
- Fix router.ts, router.py, __init__.py encoding - Use UTF-8 with BOM instead of UTF-16LE - Add SQL exports to client.ts Fixes pmxt-dev#1032
PR Review: FAILWhat This DoesAdds/export SQL result types and router Blast RadiusPython package root exports, Python router module, TypeScript router/client exports, and generated API reference docs. Consumer VerificationBefore (base branch): After (PR branch): But the changed source files contain mojibake/encoding corruption in user-facing docs/comments immediately at the top of both router modules: The diff also shows a UTF-8 BOM inserted at the beginning of the Python package root/router and TypeScript router files. I did not verify a live Test Results
Findings
PMXT Pipeline Check
Semver Impactminor -- adds a new SDK router SQL helper/type surface. RiskI could not verify the hosted SQL endpoint behavior from this environment. Before merge, the source files should be regenerated/edited with clean UTF-8, SQL types added to Python |
Fixes #1032