add support for rustls #26
Conversation
|
Hi @hungl6844 , cheers for the PR! Looks like there is a GtiHub issue blocking CI, bear with me while that gets resolved. |
|
Does this approach still use reqwest default features by default? That's my preference for now. |
ytmapi-rs/Cargo.lock
Outdated
There was a problem hiding this comment.
Please delete lockfile, not required for library (my mistake I need to add this to gitignore in future)
ytmapi-rs/Cargo.toml
Outdated
| @@ -11,7 +11,7 @@ readme = "README.md" | |||
|
|
|||
| [dependencies] | |||
| tokio = {version = "1.29.1", features = ["full"]} | |||
| reqwest = {version = "0.12.1", features = ["json"]} | |||
| reqwest = {version = "0.12.1", features = ["json"], default-features = false, optional = true} | |||
There was a problem hiding this comment.
Please add reqwest's non-tls related default features (e.g http2) https://docs.rs/reqwest/latest/reqwest/index.html
ytmapi-rs/Cargo.toml
Outdated
| @@ -11,7 +11,7 @@ readme = "README.md" | |||
|
|
|||
| [dependencies] | |||
| tokio = {version = "1.29.1", features = ["full"]} | |||
| reqwest = {version = "0.12.1", features = ["json"]} | |||
| reqwest = {version = "0.12.1", features = ["json"], default-features = false, optional = true} | |||
There was a problem hiding this comment.
Reqwest is required openssl or not, so should not be optional = true
| @@ -22,3 +22,8 @@ chrono = "0.4.31" | |||
|
|
|||
| [dev-dependencies] | |||
| pretty_assertions = "1" | |||
|
|
|||
| [features] | |||
There was a problem hiding this comment.
please add "default" feature that uses reqwest defaults (breaks youtui binary otherwise)
|
First of all, yes, by default, openssl is still used when building the binary, the feature “rusttls-tls” is required to switch away. Thanks for pointing out the issues, I didn’t even realize the binary wasn’t building (I was only using ytmapi-rs, so I never built youtui). I will implement your changes as soon as possible! |
|
Made some slight changes, if you're happy with that I'll get this merged! |
|
Go for it! |
We're in - thanks for your contrib :) |
because openssl is a pain to cross-compile for, I have added the ability for users to switch to another backend instead. since the only configuration required to do so is enabling the features in reqwest, all I had to do was add a feature in ytmapi-rs that tells reqwest to switch the backend.