[Frontend] Add opt-in native-tls crypto backend for the Rust frontend#46052
[Frontend] Add opt-in native-tls crypto backend for the Rust frontend#46052wseaton wants to merge 2 commits into
Conversation
Signed-off-by: Will Eaton <weaton@redhat.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98662ca739
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,5 +1,11 @@ | |||
| //! Minimal OpenAI-compatible HTTP server above [`vllm_chat`]. | |||
|
|
|||
| #[cfg(all(feature = "rustls", feature = "openssl"))] | |||
There was a problem hiding this comment.
In the Rust frontend CI I checked, both clippy and nextest are run with --workspace --all-features (.buildkite/scripts/run-rust-frontend-cargo-ci.sh:153-158 and :169-173), and Cargo documents --all-features as activating all available features. That means the existing pipeline enables both rustls and the new openssl feature for vllm-server, so this guard makes every CI all-features build fail before clippy/tests can run. Please structure the feature check or CI feature set so the opt-in backend does not make the repository’s standard all-features jobs uncompilable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
To use Codex here, create a Codex account and connect to github.
There was a problem hiding this comment.
Codex Review: Didn't find any major issues. Can't wait for the next one!
Reviewed commit: 9bd6c24506
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Will Eaton <weaton@redhat.com>
|
I think the direction could be simpler here, i.e., we can just target So I would prefer switching the direct Remaining transitive deps:
I've pushed a branch for this: https://github.com/vllm-project/vllm/compare/bz/remove-rustls For |
|
This pull request has merge conflicts that must be resolved before it can be |
|
@BugenZhao thanks, dropping Do you want to take this? If so I'll close! |
|
@BugenZhao some type of pre-commit hook that uses |
2 participants
Some downstream environments need to be able to build the rust frontend against the system crypto provider via
native-tls, add an optional feature to make this possible.Note: This will not be fully solved until all of the transitive deps we have are fixed to allow tls backend selection, but it is a start.