Skip to content

Commit 9af5114

Browse files
eddietejedaclaude
andcommitted
fix(query): address four code review issues in Arrow IPC path
- Route get_bytes through send_debug_bytes so Arrow requests appear in --debug output (was bypassing debug logging entirely) - Replace unwrap_or_default() with resp.bytes()? to propagate body read errors instead of silently returning empty bytes - Remove dead req.build() error arm; send_debug_bytes handles it - Change execution_time_ms to Option<u64>; async Arrow results show "—" in the table footer instead of a misleading "0 ms" - Add 5-minute polling timeout to the async execute loop with a hint to re-check via `hotdata query status <run_id>` Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 50e3439 commit 9af5114

3 files changed

Lines changed: 54 additions & 20 deletions

File tree

src/api.rs

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -282,22 +282,10 @@ impl ApiClient {
282282
pub fn get_bytes(&self, path: &str, accept: &str) -> (reqwest::StatusCode, Vec<u8>) {
283283
let url = format!("{}{path}", self.api_url);
284284
let req = self.build_request(reqwest::Method::GET, &url).header("Accept", accept);
285-
match req.build() {
286-
Ok(request) => {
287-
match self.client.execute(request) {
288-
Ok(resp) => {
289-
let status = resp.status();
290-
let bytes = resp.bytes().unwrap_or_default().to_vec();
291-
(status, bytes)
292-
}
293-
Err(e) => {
294-
eprintln!("error connecting to API: {e}");
295-
std::process::exit(1);
296-
}
297-
}
298-
}
285+
match util::send_debug_bytes(&self.client, req) {
286+
Ok(pair) => pair,
299287
Err(e) => {
300-
eprintln!("error building request: {e}");
288+
eprintln!("error connecting to API: {e}");
301289
std::process::exit(1);
302290
}
303291
}

src/query.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ pub struct QueryResponse {
1010
pub columns: Vec<String>,
1111
pub rows: Vec<Vec<Value>>,
1212
pub row_count: u64,
13-
#[serde(default)]
14-
pub execution_time_ms: u64,
13+
pub execution_time_ms: Option<u64>,
1514
pub warning: Option<String>,
1615
}
1716

@@ -127,7 +126,7 @@ fn arrow_ipc_to_query_response(bytes: Vec<u8>, result_id: String) -> QueryRespon
127126
columns,
128127
rows,
129128
row_count,
130-
execution_time_ms: 0,
129+
execution_time_ms: None,
131130
warning: None,
132131
}
133132
}
@@ -182,9 +181,20 @@ pub fn execute(
182181

183182
let run_id = &async_resp.query_run_id;
184183
let spinner = crate::util::spinner("waiting for query...");
184+
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(300);
185185

186186
loop {
187187
std::thread::sleep(std::time::Duration::from_millis(500));
188+
if std::time::Instant::now() > deadline {
189+
spinner.finish_and_clear();
190+
use crossterm::style::Stylize;
191+
eprintln!("{}", "query timed out after 5 minutes".red());
192+
eprintln!(
193+
"{}",
194+
format!("Check status with: hotdata query status {run_id}").dark_grey()
195+
);
196+
std::process::exit(1);
197+
}
188198
let run: QueryRunResponse = api.get(&format!("/query-runs/{run_id}"));
189199
match run.status.as_str() {
190200
"succeeded" => {
@@ -312,13 +322,17 @@ pub fn print_result(result: &QueryResponse, format: &str) {
312322
.as_deref()
313323
.map(|id| format!(" [result-id: {id}]"))
314324
.unwrap_or_default();
325+
let time_part = match result.execution_time_ms {
326+
Some(ms) => format!("{ms} ms"),
327+
None => "\u{2014}".to_string(), // em dash
328+
};
315329
eprintln!(
316330
"{}",
317331
format!(
318-
"\n{} row{} ({} ms){}",
332+
"\n{} row{} ({}){}",
319333
result.row_count,
320334
if result.row_count == 1 { "" } else { "s" },
321-
result.execution_time_ms,
335+
time_part,
322336
id_part
323337
)
324338
.dark_grey()
@@ -327,3 +341,4 @@ pub fn print_result(result: &QueryResponse, format: &str) {
327341
_ => unreachable!(),
328342
}
329343
}
344+

src/util.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,37 @@ pub fn send_debug(
132132
send_debug_with_redaction(client, builder, body_for_log, &[])
133133
}
134134

135+
/// Like `send_debug` but for binary (non-UTF-8) responses such as Arrow IPC.
136+
/// Logs the request and response status in debug mode; prints the byte count
137+
/// instead of attempting to parse the body as JSON.
138+
pub fn send_debug_bytes(
139+
client: &reqwest::blocking::Client,
140+
builder: reqwest::blocking::RequestBuilder,
141+
) -> reqwest::Result<(reqwest::StatusCode, Vec<u8>)> {
142+
let request = builder.build()?;
143+
if is_debug() {
144+
log_request_struct(&request, None);
145+
}
146+
let resp = client.execute(request)?;
147+
let status = resp.status();
148+
let bytes = resp.bytes()?;
149+
if is_debug() {
150+
use crossterm::style::Stylize;
151+
let status_str = format!(
152+
"<<< {} {}",
153+
status.as_u16(),
154+
status.canonical_reason().unwrap_or("")
155+
);
156+
if status.is_success() {
157+
eprintln!("{}", status_str.dark_green());
158+
} else {
159+
eprintln!("{}", status_str.dark_red());
160+
}
161+
eprintln!("{}", format!("[binary: {} bytes]", bytes.len()).dark_grey());
162+
}
163+
Ok((status, bytes.to_vec()))
164+
}
165+
135166
/// Like `send_debug` but masks the named JSON keys in the printed
136167
/// response body. The returned body string is always unredacted.
137168
pub fn send_debug_with_redaction(

0 commit comments

Comments
 (0)