-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[ci] Use env var to control whether to use S3 bucket in CI #13634
Conversation
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
vllm/utils.py
Outdated
@@ -2268,3 +2268,73 @@ def wrapped_init(self, *args, **kwargs) -> None: | |||
|
|||
type.__setattr__(cls, '__init__', wrapped_init) | |||
return cls | |||
|
|||
|
|||
MODELS_ON_S3 = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move them to vllm/test_utils.py
?
vllm/engine/arg_utils.py
Outdated
@@ -1124,6 +1125,10 @@ def create_engine_config(self, | |||
f", but got {self.cpu_offload_gb}") | |||
|
|||
device_config = DeviceConfig(device=self.device) | |||
if envs.VLLM_CI_USE_S3 and self.model in MODELS_ON_S3: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the change looks good to me.
.buildkite/test-pipeline.yaml
Outdated
@@ -53,13 +53,13 @@ steps: | |||
- tests/standalone_tests/lazy_imports.py | |||
commands: | |||
- python3 standalone_tests/lazy_imports.py | |||
- pytest -v -s mq_llm_engine # MQLLMEngine | |||
- pytest -v -s async_engine # AsyncLLMEngine | |||
- VLLM_CI_USE_S3=1 pytest -v -s mq_llm_engine # MQLLMEngine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove the change before merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approve to unblock merge. please fix comments before merge.
This pull request has merge conflicts that must be resolved before it can be |
pre-commit error also happens on main |
This pull request has merge conflicts that must be resolved before it can be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left two comments - PTAL
|
arg_utils
where if the model is in S3 allowlist and env var is on, model name is changed to S3 and load format is changed to Run AI