Skip to content

Conversation

DarkLight1337
Copy link
Member

@DarkLight1337 DarkLight1337 commented Jul 20, 2024

This PR adds vllm.connections.HTTP_CONNECTION so that we can reuse the same sync/async client session. It also encapsulates the boilerplate code for common uses of requests and aiohttp libraries.

Copy link

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which consists a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of default ones by unblocking the steps in your fast-check build on Buildkite UI.

Once the PR is approved and ready to go, please make sure to run full CI as it is required to merge (or just use auto-merge).

To run full CI, you can do one of these:

  • Comment /ready on the PR
  • Add ready label to the PR
  • Enable auto-merge.

🚀

@DarkLight1337 DarkLight1337 requested a review from ywang96 July 20, 2024 03:39
@ywang96
Copy link
Member

ywang96 commented Jul 20, 2024

@simon-mo WDYT of this PR? I think it makes sense to me to unify the management of HTTP connections.

@simon-mo
Copy link
Collaborator

Code style wise, I would prefer something like get_global_http_client get_global_async_http_client instead of a seemingly constant variable.

But good direction!

@DarkLight1337
Copy link
Member Author

Code style wise, I would prefer something like get_global_http_client get_global_async_http_client instead of a seemingly constant variable.

But good direction!

See if f4cfb65 suits you.

@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Jul 20, 2024
@DarkLight1337
Copy link
Member Author

Any update? @simon-mo

@simon-mo simon-mo merged commit 97234be into vllm-project:main Jul 23, 2024
@DarkLight1337 DarkLight1337 deleted the http-connection branch July 23, 2024 04:34
xjpang pushed a commit to xjpang/vllm that referenced this pull request Jul 24, 2024
@hmellor
Copy link
Member

hmellor commented Jul 26, 2024

I'm not 100% sure how, but this PR appears to have broken the dev documentation.

Setup environment:

cd docs
python -m venv venv
. venv/bin/activate
pip install -r requirements-docs.txt

Start HTTP server:

python -m http.server -d build/html/

Checkout commit prior to this PR and build:

git checkout c051bfe4eb77b82eba90504360bbd4e61d9e489a
make clean && make html

Navigate to http://localhost:8001/dev/sampling_params.html and see a populated page.

Checkout merge commit from this PR and build:

git checkout 97234be0ec67f48ed5e65bc0290f329dfb33798e
make clean && make html

Navigate to http://localhost:8001/dev/sampling_params.html and see an empty page.

Alvant pushed a commit to compressa-ai/vllm that referenced this pull request Oct 26, 2024
LeiWang1999 pushed a commit to LeiWang1999/vllm-bitblas that referenced this pull request Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants