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
[ENHANCEMENT] Add pass through features for reqwest #44
Comments
Thanks for opening @mwilliammyers. I hope you don't mind, but I have some questions about pass through features, as I'm not that familiar with this convention. Is there a good resource where I can learn more? Is it common to allow pass through features in this manner? What are the benefits in doing so?
These features aren't needed by the client; does it make sense to also make them pass through?
Is |
No problem! They are mentioned briefly in the Cargo.toml manifest format docs. There are some here for elastic-rs/elastic. The main benefit of using them allows the end user more control over the dependencies. It is fairly common to see this feature used for popular crates, like
Really I am just reaching for low-hanging "nice-to-haves" 😃 |
Thanks for the links @mwilliammyers. Is there a way to specify that some dependency features are required, whilst some are optional based on being opted into through passthrough features? It's not clear to me reading the manifest format docs how this should be constructed. For example, for the [features]
default = ["native-tls"]
default-tls = ["reqwest/default-tls"]
native-tls = ["reqwest/native-tls"]
rustls-tls = ["reqwest/rustls-tls"]
cookies = ["reqwest/cookies"]
socks = ["reqwest/socks"]
[dependencies]
# required features
reqwest = { version = "^0.10", features = ["gzip", "json" ] } Should the reqwest dependency also specify all of the optional features, and they'll only be included if they're features opted into in elasticsearch? I think I could solve it by making |
Good question. Off the top of my head that looked correct but I wasn't 100% sure, so I used the above in a test project and it compiled just fine. When I run |
This commit passes through features of the reqwest dependency, allowing end users more control over dependencies Closes #44
This commit passes through the following features of the reqwest dependency: - native-tls (enabled by default): Enables TLS functionality provided by native-tls. passes through to reqwest/native-tls and is set as a default feature. - rustls-tls: Enables TLS functionality provided by rustls. passes through to reqwest/rustls-tls allowing end users more control over dependencies Attribute source code and tests that require features of the elasticsearch package to be enabled. Introduce ClientCertificate enum to differentiate between PKCS#12 archives and PEM certs, which are available only when native-tls and rustls-tls features are enabled, respectively. ClientCertificate enum takes bytes representing a certificate as opposed to a reqwest Identity because Identity does not implement Clone, and cannot be dereferenced from a Credentials::Certificate variant when building a Transport because other variants are passed to the built Transport. Closes #44
I've implemented I've been thinking about having pass-through features for the other features of reqwest, and it doesn't feel right to expose these in the client. On one hand, I can see how they could help with compilation, but on the other, the elasticsearch crate would then be exposing features of dependencies that are not relevant to the crate. |
Yeah, sounds good to me. |
In an effort to reduce dependency duplication as
reqwest
is a very commonly used crate; it would be handy to have some features we can pass through toreqwest
.Some candidates are:
rustls-tls = ["reqwest/rustls-tls"]
cookies = ["reqwest/cookies"]
socks = ["reqwest/socks"]
Do we need the
native-tls
feature specifically? It looks like it adds some more features vs.default-tls
? Canrustls-tls
be used instead?rusttls-tls
makes building a static binary via musl libc much easier...The text was updated successfully, but these errors were encountered: