feat(query,write): HTTP wire adapter + query/write commands + 4 output formats (PR2)#1
Merged
Merged
Conversation
…formats
PR2 of the phased arcctl build-out. Adds `internal/client/` (the
typed HTTP adapter for /api/v1/query, /api/v1/query/arrow,
/api/v1/write/line-protocol), the `arcctl query` and `arcctl write`
cobra commands, and a query-result renderer covering table, JSON,
CSV, and Arrow IPC output.
Highlights
- `arcctl query "SELECT ..."` with -f/stdin/positional SQL input
- 4 output formats: table (default), json, csv, arrow (binary IPC on
stdout; server execution-time read from Arc's response trailer)
- `arcctl write` reads stdin or -f; --precision ns|us|ms|s validated
client-side; body is streamed, never buffered (large file friendly)
- buildClient() centralises the connection-precedence resolution
shared by both commands; --insecure / insecure_tls only warn on
https:// (no-op + suppressed on http)
- Bounded response body reads (64 MiB cap on query JSON), bounded
error-body reads (64 KiB) to keep a misbehaving proxy from OOMing
the CLI
- Empty result on -o table now prints "(0 rows)" instead of nothing
- --timeout 0s now errors instead of producing an already-cancelled
context
Verification
- Unit tests: 9 client tests (httptest) + 8 renderer tests, race-clean
- End-to-end smoke against arc serve: write stdin LP, write -f, query
-o table/json/csv/arrow (Arrow IPC trailer round-tripped; 456-byte
stream from real DuckDB validated)
- Internal review (matrix + single deep reviewer): 2 High addressed
(trailer test shape, mid-stream interruption UX), 3 Medium
addressed (timeout guard, stderr routing through cmd.ErrOrStderr,
nil QueryResult guard), 2 Style addressed (Close() docstring
overclaim, isPipe comment clarity); 1 Medium deferred (error-shape
evolution to json.RawMessage; tracked for when Arc's error
contract changes)
Companion docs PR lands in docs.basekick.net (docs/cli/{index,
connections,query,write}.md).
Member
Author
|
@gemini-code-assist review please |
3 tasks
xe-nvdk
pushed a commit
that referenced
this pull request
Jun 1, 2026
) Two findings from Gemini's review on #3, both Medium. #1 — `--skip-rows` accepts negative values silently A negative value passes through to `ImportCSV`, then gets dropped by the `opts.SkipRows > 0` gate in the client. User typed `--skip-rows -5`, nothing happens, no error. Now validated in the command's RunE before any client call. Regression test driven through cobra so the wired flag path is covered end-to-end; mutation-verified by disabling the guard (downstream "no database" check fires instead). #2 — `mw.Close()` on error paths writes trailing boundary into broken pipe In the multipart goroutine, the error paths used to call `mw.Close()` *before* `pw.CloseWithError(err)`. `multipart.Writer.Close()` writes the trailing boundary `--<boundary>--\r\n` into the pipe — but the preceding content is already known-bad (CreateFormFile failed, or io.Copy errored mid-file). Result: a multipart payload that's syntactically valid but semantically truncated, confusing for a reader attempting to make sense of partial data. Now: error paths skip `mw.Close()` entirely and signal the error on the pipe; happy path still closes the writer normally to emit the trailing boundary.
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
1 participant
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.
Summary
internal/client/— typed HTTP adapter for/api/v1/query,/api/v1/query/arrow, and/api/v1/write/line-protocol.arcctl queryandarcctl writecommands via cobra.arcctl queryaccepts SQL as positional arg,-f file.sql, or stdin pipe.arcctl writereads stdin or-f file.lp, streams the body (large-file friendly), validates--precision ns|us|ms|sclient-side.buildClient()enforces the connection-precedence rules from PR1 and emits TLS-skip warnings only onhttps://(no-op + suppressed onhttp://).Test plan
go test -race -count=1 ./...— 17 new tests acrossinternal/client/andinternal/output/gofmt -l . && go vet ./...cleanarc serve:config create→writevia stdin LP →write -fwith--precision ms→query -o table/json/csv/arrowall verified. Arrow IPC produced a valid 456-byte stream from real DuckDB and theArc-Execution-Time-Mstrailer round-tripped (server execution1msshowed on stderr).--endpointonly) → caught client-side, bad--precision→ caught client-side,--insecurewarning fires onhttps://and is suppressed onhttp://.(0 rows)printed instead of blank output.Internal review
Matrix + single deep reviewer per
.claude/CLAUDE.md. Findings addressed pre-commit:http.TrailerPrefix(the production pattern)arrow: stream interrupted after N bytesline to stderr alongside the error--timeout 0snow errors instead of producing an instantly-cancelled contextcmd.ErrOrStderr()for test captureRenderQueryResultnil-guardClose()doc no longer overclaims idempotency;isPipecomment clarifies the bytes.Buffer-in-tests rationalejson.RawMessage(tracked for when Arc's error contract changes)Companion docs PR
User-facing docs land in
docs.basekick.netatdocs/cli/{index,connections,query,write}.mdper the per-PR-docs rule.🤖 Generated with Claude Code