Skip to content

🔥🔧 Remove environment variables specific to hardware conf #229

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

Merged
merged 1 commit into from
Jun 13, 2025

Conversation

gkumbhat
Copy link
Contributor

Changes

Following environment variables are quite specific to the hardware and platform vllm-spyre is being run on. Therefore, ideally we should rely on the "reasonable" defaults for this from underlying stack rather than changing those defaults in vLLM.

Env variables:

  • FLEX_UNLINK_DEVMEM
  • FLEX_RDMA_MODE_FULL

Copy link

👋 Hi! Thank you for contributing to vLLM support on Spyre.
Just a reminder: Make sure that your code passes all the linting checks, otherwise your PR won't be able to be merged. To do so, first install the linting requirements, then run format.sh and commit the changes. This can be done with uv directly:

uv sync --frozen --group lint --active --inexact

Or this can be done with pip:

uv pip compile --group lint > requirements-lint.txt
pip install -r requirements-lint.txt
bash format.sh

Now you are good to go 🚀

Copy link
Collaborator

@prashantgupta24 prashantgupta24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but we need to decide which Dockerfile we want to keep as a single source of truth.

@joerunde
Copy link
Collaborator

LGTM, 🤞 that soon we'll have public docs to link out to about these so that users actually know how to set them when they're needed

@joerunde joerunde merged commit 48e8e85 into vllm-project:main Jun 13, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants