Switch to batched OSV queries for uv audit#18394
Conversation
Signed-off-by: William Woodruff <william@astral.sh>
crates/uv-audit/src/service/osv.rs
Outdated
| let vuln = self.fetch_vuln(&id).await?; | ||
| Ok::<(String, Vulnerability), Error>((id, vuln)) | ||
| }) | ||
| .buffer_unordered(usize::MAX); |
There was a problem hiding this comment.
Flagging: I wasn't sure if it makes sense to plumb Concurrency::downloads as a limit here.
There was a problem hiding this comment.
Yes, that limit should apply in all locations where we do network requests.
There was a problem hiding this comment.
Done! I need to separately plumb the concurrency from the CLI, I'll do that in a follow-up.
konstin
left a comment
There was a problem hiding this comment.
We need the cocnurrency limit, otherwise it's good to go!
crates/uv-audit/src/service/osv.rs
Outdated
| let vuln = self.fetch_vuln(&id).await?; | ||
| Ok::<(String, Vulnerability), Error>((id, vuln)) | ||
| }) | ||
| .buffer_unordered(usize::MAX); |
There was a problem hiding this comment.
Yes, that limit should apply in all locations where we do network requests.
crates/uv-audit/src/service/osv.rs
Outdated
| //! | ||
| //! [OSV]: https://osv.dev/ | ||
|
|
||
| use std::collections::{HashMap, HashSet}; |
There was a problem hiding this comment.
I think we generally prefer using rustc_hash::{FxHashMap, FxHashSet} in uv.
There was a problem hiding this comment.
Yep, I'll replace. It probably makes sense to slowly replace any pre-existing callsites for these and eventually ban their use via clippy?
Signed-off-by: William Woodruff <william@astral.sh>
Signed-off-by: William Woodruff <william@astral.sh>
Signed-off-by: William Woodruff <william@astral.sh>
Summary
This switches us to OSV's batch query API for vulnerability ID lookups, which can then be used to concurrently fetch the actual full finding responses.
In my local testing, this yields significant speedups: from 23s on main (before this PR) with a small project (~70 deps) to 950ms with this PR.
WIP, I want to think through this approach a little more.See #18119
Test Plan
Added new unit tests.