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

[Bug] Avoid including sensitive info in Dockerfile ENV #487

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

zhangjyr
Copy link
Collaborator

@zhangjyr zhangjyr commented Dec 5, 2024

Pull Request Description

This PR move sensitive info from Dockerfile ENV to config.json. In addition, Python 3.10 slim base image missing zScaler root CA, which causes Tokenizer to fail to download llama2 tokenizer. zScaler root CA is added manually to make it work.

Related Issues

Resolves: #483

Important: Before submitting, please complete the description above and review the checklist below.


Contribution Guidelines (Expand for Details)

We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:

Pull Request Title Format

Your PR title should start with one of these prefixes to indicate the nature of the change:

  • [Bug]: Corrections to existing functionality
  • [CI]: Changes to build process or CI pipeline
  • [Docs]: Updates or additions to documentation
  • [API]: Modifications to aibrix's API or interface
  • [CLI]: Changes or additions to the Command Line Interface
  • [Misc]: For changes not covered above (use sparingly)

Note: For changes spanning multiple categories, use multiple prefixes in order of importance.

Submission Checklist

  • PR title includes appropriate prefix(es)
  • Changes are clearly explained in the PR description
  • New and existing tests pass successfully
  • Code adheres to project style and best practices
  • Documentation updated to reflect changes (if applicable)
  • Thorough testing completed, no regressions introduced

By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.

Add missing zscaler root CA to image for huggingface lib to download tokenizer model successfully.
@zhangjyr zhangjyr requested review from Jeffwan and nwangfw December 5, 2024 07:21
@zhangjyr zhangjyr changed the title Avoid including sensitive info in Dockerfile ENV [Bug] Avoid including sensitive info in Dockerfile ENV Dec 5, 2024
@zhangjyr zhangjyr added this to the v0.2.0 milestone Dec 5, 2024
@nwangfw
Copy link
Collaborator

nwangfw commented Dec 5, 2024

Looks good to me.

@Jeffwan
Copy link
Collaborator

Jeffwan commented Dec 5, 2024

I thought we would still use a public available tokenizer instead. Now it leaks the hugging face token. I suggest to use different one instead. Does an open source model like https://huggingface.co/Qwen/Qwen2.5-Coder-7B-Instruct make sense ?

@Jeffwan Jeffwan added kind/bug Something isn't working priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. area/heterogeneous labels Dec 5, 2024
@zhangjyr
Copy link
Collaborator Author

zhangjyr commented Dec 5, 2024

I thought we would still use a public available tokenizer instead. Now it leaks the hugging face token. I suggest to use different one instead. Does an open source model like https://huggingface.co/Qwen/Qwen2.5-Coder-7B-Instruct make sense ?

Sorry, The hugging face token is not supposed to be there. I'll remove it. We can use other models later. But for now, Vidur only supports limited models; it needs to be re-profiled to adapt to non-gated models. Currently, only Qwen/Qwen-72B is on the support list, but it might be too large for local development.

@Jeffwan
Copy link
Collaborator

Jeffwan commented Dec 5, 2024

That makes sense. Let's stick to current plan and revisit the supported models later.

@Jeffwan Jeffwan merged commit 93603ff into main Dec 5, 2024
2 checks passed
@Jeffwan Jeffwan deleted the issues/483_SecretsUsedInArgOrEnv branch December 5, 2024 19:18
gangmuk pushed a commit that referenced this pull request Jan 25, 2025
* Move huggingface_token to config.json
Add missing zscaler root CA to image for huggingface lib to download tokenizer model successfully.

* Remove huggingface token

---------

Co-authored-by: Jingyuan Zhang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/heterogeneous kind/bug Something isn't working priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SecretsUsedInArgOrEnv on building mock app.
3 participants