Skip to content
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

Implement warp::reply::file for dynamic file service #1049

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Molkars
Copy link

@Molkars Molkars commented Jul 2, 2023

This pull request implements warp::reply::file, warp::header::conditionals, and publishes warp::header::Conditionals. I also internally moves Conditionals to what I believe is a more fitting place (from fs to header).

Fixes #171
Supersedes #924

@seanmonstar
Copy link
Owner

Awesome work, thanks! I think I have mainly one suggestion: make Conditionals optional. reply::file(path) could return File, which has an inherent method conditionals() that takes the conditional arguments. What do you think?

@Molkars
Copy link
Author

Molkars commented Jul 4, 2023

I'm not sure that's very feasible since we return a future dependent on the value initially passed in. We could have a reply::conditional_file though

@mvdschee
Copy link

I've been using Molkar's version for a while now, and it works perfectly for my needs. How can we make this PR land into the main version?

@Latawiec
Copy link

Any updates here? I'd love to have this feature

@alexander-camuto
Copy link

Any updates here? I'd love to have this feature

same

@nickbp
Copy link
Contributor

nickbp commented Dec 30, 2023

I'd independently hit this very issue when trying to build a file server that couldn't serve files. I'd imagined that warp would have the building blocks available to allow DIYing this, but AFAICT the only solution right now is to copy/paste a bunch of pub(crate) code out of warp. I got pretty far in doing that, but realized that the exercise had become more hassle than it was worth.

If anyone else stumbles upon this issue after hitting a brick wall like I had, I ended up just switching to using this instead: https://api.rocket.rs/master/rocket/fs/struct.FileServer.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add reply::file(path) helper
6 participants