Skip to content
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

feat(venv): support pip options #1052

Merged
merged 5 commits into from
Aug 2, 2024
Merged

feat(venv): support pip options #1052

merged 5 commits into from
Aug 2, 2024

Conversation

rickzx
Copy link
Contributor

@rickzx rickzx commented Aug 1, 2024

  • Support pip options when creating venv. Now requirements.txt can have options like
--extra-index-url https://flashinfer.ai/whl/cu121/torch2.3
...
  • Support setting env vars if specified in bento.yaml. This is needed for, e.g., running gemma-2 model with
VLLM_ATTENTION_BACKEND=FLASHINFER
  • Reformat all Python files with black

@rickzx rickzx requested a review from aarnphm as a code owner August 1, 2024 00:14
@rickzx rickzx requested a review from bojiang August 1, 2024 00:14
Copy link
Collaborator

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

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

shouldn't ruff format be sufficient enough?

venv = ensure_venv(bento)
cmd, env, cwd = _get_serve_cmd(bento, port=port)
output(f'Access the Chat UI at http://localhost:{port}/chat (or with you IP)')
run_command(cmd, env=env, cwd=cwd, venv=venv)


def prep_env_vars(bento: BentoInfo):
import os
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should move this to top of files.

@aarnphm aarnphm changed the title Support pip options when creating venv. Support extra env_vars. Reformat Python files feat(venv): support pip options Aug 1, 2024
Signed-off-by: Rick Zhou <rickzhoucmu@gmail.com>
@rickzx
Copy link
Contributor Author

rickzx commented Aug 1, 2024

shouldn't ruff format be sufficient enough?

Yeah, i realized the CI is already doing this. No need for black format

@@ -16,6 +28,7 @@ def _get_serve_cmd(bento: BentoInfo, port: int = 3000):


def serve(bento: BentoInfo, port: int = 3000):
prep_env_vars(bento)
venv = ensure_venv(bento)
cmd, env, cwd = _get_serve_cmd(bento, port=port)
Copy link
Member

Choose a reason for hiding this comment

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

It seems the prepared env isn't applied to bentoml serve? @rickzx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is applied to bentoml serve. The run_command function explicitly copies the env vars from the current process to the new process (even if it doesn't, the default subprocess will also inherit env from the current process)

@rickzx rickzx merged commit c6e4e69 into main Aug 2, 2024
3 checks passed
@rickzx rickzx deleted the pr-rick-gemma2 branch August 2, 2024 08:43
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