-
Notifications
You must be signed in to change notification settings - Fork 224
Add ZIP download endpoint #410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
55a0a59
to
12e1b19
Compare
IMO ideally we'd have a combined dropdown + button, where if you click the button you get the ZIP by default, but there's advanced options to get PCAP and QMDL. I tried and failed to implement that using |
2337014
to
20308f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is great! i like it a lot more than my client-side solution. just have a few nits/comments
bin/src/server.rs
Outdated
|
||
tokio::spawn(async move { | ||
let result: Result<(), Error> = async { | ||
let mut zip2 = ZipFileWriter::with_tokio(writer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why zip2
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftover from the porting effort from zip
to async-zip
. renamed to just zip
.take(qmdl_size_bytes as u64) | ||
}; | ||
|
||
copy(&mut qmdl_file, &mut entry_writer).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh copy
seems handy
bin/src/server.rs
Outdated
{ | ||
let entry = | ||
ZipEntryBuilder::new(format!("{qmdl_idx}.qmdl").into(), Compression::Stored); | ||
let mut entry_writer = zip2.write_entry_stream(entry).await?.compat_write(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not sure i understand what compat_write()
is doing -- honestly it was hard even finding where the method's coming from. maybe add a comment explaining it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops i meant to request changes
fix #409
don't know what to do with the other buttons. just remove?