[Core] Add Downloader implementation for runtime #96
Conversation
Jeffwan
changed the title
Jeffwan
changed the title
| @@ -131,9 +134,20 @@ def download_model(self, local_path: Optional[str]): | |||
| if self._is_directory(): | |||
| self.download_directory(model_path) | |||
| else: | |||
| self.download(self.model_uri, model_path) | |||
| self.download( | |||
| filename=self.model_uri, | |||
There was a problem hiding this comment.
I feel the naming is a little bit weird. file should be subset of the model_uri.
There was a problem hiding this comment.
Yes, I also feel a bit awkward here. For TOS, bucket_name is in host_name of model_uri . For AWS, bucket_name is in the path of the model_uri. It is difficult to distinguish which subset of the model_uri is obtained in the base class. Or we should rename filename？
There was a problem hiding this comment.
you mean TOS uri is like bucket-name-tos-beijing/path and S3 is s3://bucket-name/path? for the internal methods, I suggest to have two variables like bucket and path so we can construct the path as needed. but for the user's input. they probably give the URI directly, right? is there a way to split the string into bucket and path?
| ) | ||
|
|
||
| # create completed file to indicate download completed | ||
| completed_file = model_path.joinpath(".completed") |
There was a problem hiding this comment.
where do you use it for completion check?
There was a problem hiding this comment.
What I want is to first execute an entrypoint script in the main container to check if the file .completed exists in the model directory, and then execute the subsequent model startup logic. This can ensure the order of download and startup.
There was a problem hiding this comment.
Let's say we use the vLLM, in that case, we need to change the entrypoint or add a wrapper?
There was a problem hiding this comment.
Yes, it's like using script content like the following as an entrypoint
#!/bin/bash
MODEL_PATH="/path/to/model"
while true; do
if [ -e "$FILEPATH/.completed" ]; then
echo "File exists. Continuing execution..."
break
else
sleep 1
fi
done
python3 -m vllm.entrypoints.openai.api_server $@ --model $MODEL_PATH
| ) | ||
| self.bucket_name = bucket_name | ||
| self.bucket_path = bucket_path | ||
| self.client = tos.TosClientV2(ak, sk, endpoint, region) |
There was a problem hiding this comment.
just curious, tos claims they are s3 api compatible. can we use s3 sdk instead? do you know the best practice?
There was a problem hiding this comment.
I feel like it's difficult to find documents related to TOS, so I'll try to find them.
There was a problem hiding this comment.
no worries, this should be low priority. I am just curious about TOS's maturity
| def download( | ||
| self, | ||
| local_path: Path, | ||
| bucket_path: str, |
There was a problem hiding this comment.
em. this is ok at this moment. I feel it's a little bit weird to have both of them. we can discuss it in future issues/prs.
|
Looks good to me and feel free to merge it |
|
I will merge it |
* feat: add huggingface downloader implement * feat: add tos downloader implement * fix: fix download single file from tos * feat: add s3 downloader implement * feat: add progress bar into s3 and tos downloader * ci: add ruff format check * style: remove unused import * fix: huggingface downloader init model name * refact: refact tos model uri use tos protocol * refact: use bucket path * style: fix code style * refact: remove .complete file
Pull Request Description
Support download model file from S3, huggingFace and TOS
Related Issues
Resolves: #81
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
By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.