ingestd/vectord: remove two fragile unwraps on Option paths
Some checks failed
lakehouse/auditor 1 blocking issue: todo!() macro call in tests/real-world/scrum_master_pipeline.ts
Some checks failed
lakehouse/auditor 1 blocking issue: todo!() macro call in tests/real-world/scrum_master_pipeline.ts
Both were technically safe — guarded above by map_or(true, ...) and
Some(entry) assignment respectively — but relied on multi-line
invariants that a future refactor could easily break.
- ingestd/watcher.rs:80: path.file_name().unwrap() on a path that
was already checked via map_or(true, ...) two lines up. Fix:
let-else binds filename once, no double lookup, no unwrap.
- vectord/promotion.rs:145: file.current.as_ref().unwrap() called
TWICE on the same line to log config + trial_id. Guard via
`if let Some(cur) = &file.current` so the log gracefully skips
if the invariant ever breaks instead of panicking at runtime.
Both are drop-in semantically: happy path identical, error path now
graceful-skip instead of panic. Workspace warnings still at 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
a934a76988
commit
12e615bb5d
@ -72,12 +72,14 @@ async fn process_inbox(
|
|||||||
|
|
||||||
let path = entry.path();
|
let path = entry.path();
|
||||||
|
|
||||||
// Skip directories and hidden files
|
// Skip directories and hidden files. Bind filename once via
|
||||||
if path.is_dir() || path.file_name().map_or(true, |n| n.to_string_lossy().starts_with('.')) {
|
// let-else so the subsequent use is unwrap-free — previous
|
||||||
continue;
|
// version relied on a map_or guard above + an .unwrap() here
|
||||||
}
|
// being consistent, which is a fragile invariant.
|
||||||
|
if path.is_dir() { continue; }
|
||||||
let filename = path.file_name().unwrap().to_string_lossy().to_string();
|
let Some(fn_os) = path.file_name() else { continue; };
|
||||||
|
let filename = fn_os.to_string_lossy().to_string();
|
||||||
|
if filename.starts_with('.') { continue; }
|
||||||
tracing::info!("watcher: found new file '{}'", filename);
|
tracing::info!("watcher: found new file '{}'", filename);
|
||||||
|
|
||||||
// Read file
|
// Read file
|
||||||
|
|||||||
@ -131,6 +131,11 @@ impl PromotionRegistry {
|
|||||||
file.history.drain(0..drop);
|
file.history.drain(0..drop);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Bind `entry` ref-captured for the log line below so the log
|
||||||
|
// doesn't double-unwrap file.current — entry is Some-by-construction
|
||||||
|
// at the function boundary; past versions reached in via
|
||||||
|
// `.as_ref().unwrap()` twice, which compiled but would panic if
|
||||||
|
// the construction above ever changed.
|
||||||
file.current = Some(entry);
|
file.current = Some(entry);
|
||||||
file.index_name = index_name.to_string();
|
file.index_name = index_name.to_string();
|
||||||
|
|
||||||
@ -140,10 +145,12 @@ impl PromotionRegistry {
|
|||||||
ops::put(&store, &key, json.into()).await?;
|
ops::put(&store, &key, json.into()).await?;
|
||||||
|
|
||||||
self.cache.write().await.insert(index_name.to_string(), file.clone());
|
self.cache.write().await.insert(index_name.to_string(), file.clone());
|
||||||
|
if let Some(cur) = &file.current {
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
"promoted '{}' to config {:?} (trial={})",
|
"promoted '{}' to config {:?} (trial={})",
|
||||||
index_name, file.current.as_ref().unwrap().config, file.current.as_ref().unwrap().trial_id,
|
index_name, cur.config, cur.trial_id,
|
||||||
);
|
);
|
||||||
|
}
|
||||||
Ok(file)
|
Ok(file)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user