refactor(appkit): split SQLWarehouseConnector into submit/get/poll/transform#375
Draft
ditadi wants to merge 1 commit into
Draft
refactor(appkit): split SQLWarehouseConnector into submit/get/poll/transform#375ditadi wants to merge 1 commit into
ditadi wants to merge 1 commit into
Conversation
This was referenced May 11, 2026
6efbe15 to
a8b7010
Compare
4e15460 to
9e0ea14
Compare
…ansform
`SQLClient.executeStatement` was a single block: submit the SQL, poll
until terminal, transform the Arrow payload to JSON. Splitting it into
four narrower public APIs lets durable executors compose them without
holding the orchestrator open across the wait:
- `submitStatement(sql, params, opts)` — POST `/sql/statements`,
returns the raw initial response. Adds a dedicated `sql.submit` span.
- `getStatement(id)` — GET `/sql/statements/{id}`, single status read.
- `pollStatement(id, opts)` — block until the statement reaches a
terminal state (SUCCEEDED / FAILED / CANCELED / CLOSED), respecting
the same timeout, signal, and error semantics the old monolithic
method had.
- `transformResult(response)` — Arrow → JSON row transform, no I/O.
`executeStatement(...)` is preserved and now composes the four publics
(`submit` → `poll` → `transform`). No private wrapper-only helpers
remain. Every error path, abort branch, and status state machine of
the old method is exercised by the new per-API test suites (21 new
tests against `submit` / `get` / `poll` / `transform`).
Motivation (documented inline in JSDoc): durable callers — e.g. a
future TaskFlow-based analytics handler — emit a `statement_submitted`
event with the warehouse-side statement ID right after `submitStatement`
returns, so on crash recovery they can re-attach via `pollStatement`
without re-running the SQL. The TaskFlow integration itself is not in
this PR.
`executeStatement`'s contract is unchanged; analytics (the only external
caller) keeps working without modification. The added `sql.submit` span
is purely additive for OTLP collectors.
Verified: pnpm -r typecheck, pnpm build, full pnpm test
(122 files, 2276 tests) all green.
Signed-off-by: ditadi <victordperd@gmail.com>
9e0ea14 to
b1491b8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🥞 Stacked PR
Use this link to review incremental changes.
SQLClient.executeStatementwas a single block: submit the SQL, poll until terminal, transform the Arrow payload to JSON. Splitting it into four narrower public APIs lets durable executors compose them without holding the orchestrator open across the wait:submitStatement(sql, params, opts)— POST/sql/statements, returns the raw initial response. Adds a dedicatedsql.submitspan.getStatement(id)— GET/sql/statements/{id}, single status read.pollStatement(id, opts)— block until the statement reaches a terminal state (SUCCEEDED/FAILED/CANCELED/CLOSED), respecting the same timeout, signal, and error semantics the old monolithic method had.transformResult(response)— Arrow → JSON row transform, no I/O.executeStatement(...)is preserved and now composes the four publics (submit→poll→transform). No private wrapper-only helpers remain. Every error path, abort branch, and status state machine of the old method is exercised by the new per-API test suites (21 new tests againstsubmit/get/poll/transform).Motivation (documented inline in JSDoc): durable callers — e.g. a future TaskFlow-based analytics handler — emit a
statement_submittedevent with the warehouse-side statement ID right aftersubmitStatementreturns, so on crash recovery they can re-attach viapollStatementwithout re-running the SQL. The TaskFlow integration itself is not in this PR.executeStatement's contract is unchanged; analytics (the only external caller) keeps working without modification. The addedsql.submitspan is purely additive for OTLP collectors.Verified:
pnpm -r typecheck,pnpm build, fullpnpm test(122 files, 2276 tests) all green.Signed-off-by: ditadi victordperd@gmail.com