Conversation
There was a problem hiding this comment.
Review of the Solana CAR download script and solana_compare tracing improvements.
Key findings (by priority):
-
Performance — blocking sync I/O in async context:
fs_err::exists,fs_err::create_dir_all, andfs_err::metadataare blocking calls on async worker threads. Usetokio::fsequivalents. -
Panic branch:
.expect("invalid range header")should return an error instead of panicking. -
Durability for resume:
flush()doesn't guarantee data persistence to disk. Usesync_all()since the resume logic depends on accurate on-disk file sizes. -
No integrity verification: Only file size is checked for completeness. A corrupted or truncated download would not be detected.
-
No reqwest timeout: Default client has no connect/read timeout, meaning a stalled connection could hang indefinitely without triggering a retry.
-
Guideline violations: Missing doc comments on
CarDownloadError, abbreviated log field names, non-standard duration logging format, and#[from]usage on error variants. -
DRY:
env_filter_or_infois duplicated verbatim across two example files.
b20c5f0 to
415df97
Compare
leoyvens
left a comment
There was a problem hiding this comment.
Object storage support will be important
No description provided.