Skip to content

Commit e2e151d

Browse files
eddietejedaclaude
andcommitted
fix: address code review feedback from post-release audit
- query.rs: remove dead AsyncResponse.status field (was the source of the dead_code compiler warning; field was printed pre-polling but is unused now that the loop reads QueryRunResponse.status) - query.rs: reorder polling loop — poll first, then check deadline and sleep; eliminates the mandatory 500ms delay before the first check and ensures timeout is detected without an extra sleep cycle - skill.rs: add 120s timeout to download_and_extract_from_url HTTP client, matching the timeout used in perform_update; prevents an indefinite hang during hotdata update if the skills download stalls - main.rs: replace chained .unwrap() with .expect() in the databases tables help path for clearer panic messages if subcommand names change Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 84095c0 commit e2e151d

3 files changed

Lines changed: 19 additions & 17 deletions

File tree

src/main.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -484,11 +484,11 @@ fn main() {
484484
let mut cmd = Cli::command();
485485
cmd.build();
486486
cmd.find_subcommand_mut("databases")
487-
.unwrap()
487+
.expect("databases subcommand not found")
488488
.find_subcommand_mut("tables")
489-
.unwrap()
489+
.expect("tables subcommand not found")
490490
.print_help()
491-
.unwrap();
491+
.expect("failed to print help");
492492
}
493493
}
494494
},

src/query.rs

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ pub struct QueryResponse {
1717
#[derive(Deserialize)]
1818
struct AsyncResponse {
1919
query_run_id: String,
20-
status: String,
2120
}
2221

2322
#[derive(Deserialize)]
@@ -184,17 +183,6 @@ pub fn execute(
184183
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(300);
185184

186185
loop {
187-
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-
}
198186
let run: QueryRunResponse = api.get(&format!("/query-runs/{run_id}"));
199187
match run.status.as_str() {
200188
"succeeded" => {
@@ -218,7 +206,7 @@ pub fn execute(
218206
eprintln!("{}", format!("query failed: {err}").red());
219207
std::process::exit(1);
220208
}
221-
"running" | "queued" | "pending" => continue,
209+
"running" | "queued" | "pending" => {}
222210
status => {
223211
spinner.finish_and_clear();
224212
use crossterm::style::Stylize;
@@ -230,6 +218,17 @@ pub fn execute(
230218
std::process::exit(2);
231219
}
232220
}
221+
if std::time::Instant::now() > deadline {
222+
spinner.finish_and_clear();
223+
use crossterm::style::Stylize;
224+
eprintln!("{}", "query timed out after 5 minutes".red());
225+
eprintln!(
226+
"{}",
227+
format!("Check status with: hotdata query status {run_id}").dark_grey()
228+
);
229+
std::process::exit(1);
230+
}
231+
std::thread::sleep(std::time::Duration::from_millis(500));
233232
}
234233
}
235234

src/skill.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,10 @@ fn download_and_extract_from_url(url: &str) -> Result<(), String> {
205205
// `resp.text()` and would corrupt the gzip stream). Log the
206206
// request line manually so `--debug` still shows the URL.
207207
crate::util::debug_request("GET", url, &[], None);
208-
let client = reqwest::blocking::Client::new();
208+
let client = reqwest::blocking::Client::builder()
209+
.timeout(std::time::Duration::from_secs(120))
210+
.build()
211+
.map_err(|e| format!("error creating HTTP client: {e}"))?;
209212
let resp = client
210213
.get(url)
211214
.send()

0 commit comments

Comments
 (0)