refactor(appkit): split SQLWarehouseConnector into submit/get/poll/transform#372
Closed
ditadi wants to merge 1 commit into
Closed
refactor(appkit): split SQLWarehouseConnector into submit/get/poll/transform#372ditadi wants to merge 1 commit into
ditadi wants to merge 1 commit into
Conversation
…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>
Contributor
Author
|
Superseded by #375 — replatformed onto |
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.
Stack
CoreServiceRegistry+ServiceLocator(layer 1, base:main)SQLWarehouseConnectorinto submit/get/poll/transform (layer 2, base: refactor(appkit): introduce CoreServiceRegistry and ServiceLocator #371)Based on #371. Review independently —
SQLClientpublic contract is unchanged.SQLClient.executeStatementwas a single block: submit the SQL, polluntil 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.submitspan.getStatement(id)— GET/sql/statements/{id}, single status read.pollStatement(id, opts)— block until the statement reaches aterminal 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 helpersremain. 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_submittedevent with the warehouse-side statement ID right after
submitStatementreturns, so on crash recovery they can re-attach via
pollStatementwithout re-running the SQL. The TaskFlow integration itself is not in
this PR.
executeStatement's contract is unchanged; analytics (the only externalcaller) keeps working without modification. The added
sql.submitspanis 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
Stack created with GitHub Stacks CLI • Give Feedback 💬