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

Use minreq instead of reqwest #917

Closed
wants to merge 1 commit into from

Conversation

aumetra
Copy link

@aumetra aumetra commented May 8, 2024

The latest version of utoipa-swagger-ui introduced a dependence on reqwest at build-time just to download a single file.

That feels unnecessarily heavy, considering it's intended to be a one-off download thing, and reqwest pulls in the entirety of hyper, tokio, mio, and all supporting libraries of that.

This PR replaces it with minreq which has a smaller dependency tree.
The difference here is that minreq buffers the entire body into memory, but I checked the size of the archive and 4MB seem like a fine amount to buffer in memory.

If it isn't, then we can switch to ureq which offers an io::Read based interface, so it streams the response (but it is a little heavier on dependencies than minreq, but not by much).

@juhaku
Copy link
Owner

juhaku commented May 14, 2024

I am not fond to accept a crate as a dependency that is not licensed either with MIT or Apache-2.0 or both of them. Also as what comes to the functionalities it is unfortunate that using async rust based crate bring a lot of dependencies. However like the reqwest crate https://docs.rs/reqwest/latest/reqwest/#proxies, the library should support proxies, more over the system proxies that are set with HTTP_PROXY and HTTPS_PROXY environment variables. This is fundamental because some people might be working from within corporation network that is behind a VPN where all the internet traffic is routed through a custom HTTP(S) proxy. If the crate does not support these by default then those users will face frustration when they cannot build their crates.

@aumetra
Copy link
Author

aumetra commented May 14, 2024

That would then be ureq. Is MIT/Apache Dual licensed and can read proxies from the environment.

Just one sidenote: the fact that anything is downloaded here at all breaks building this crate via Nix since you can't just access the network while Nix builds a crate, since that would potentially break reproducibility.

@juhaku
Copy link
Owner

juhaku commented May 14, 2024

Just one sidenote: the fact that anything is downloaded here at all breaks building this crate via Nix since you can't just access the network while Nix builds a crate, since that would potentially break reproducibility.

Yeah related to that there is ongoing discussion here #923.

@juhaku juhaku closed this Jul 24, 2024
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.

None yet

2 participants