-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
[Bugfix] Path join when building local path for S3 clone #12353
[Bugfix] Path join when building local path for S3 clone #12353
Conversation
👋 Hi! Thank you for contributing to the vLLM project. 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 do one of these:
🚀 |
d841785
to
871088e
Compare
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.
Thanks for fixing!
Please fix the pre-commit error. |
Signed-off-by: Omer Dayan (SW-GPU) <omer@run.ai>
871088e
to
557071d
Compare
…t#12353) Signed-off-by: Omer Dayan (SW-GPU) <omer@run.ai>
…t#12353) Signed-off-by: Omer Dayan (SW-GPU) <omer@run.ai>
…t#12353) Signed-off-by: Omer Dayan (SW-GPU) <omer@run.ai>
…t#12353) Signed-off-by: Omer Dayan (SW-GPU) <omer@run.ai>
…t#12353) Signed-off-by: Omer Dayan (SW-GPU) <omer@run.ai>
…t#12353) Signed-off-by: Omer Dayan (SW-GPU) <omer@run.ai> Signed-off-by: Isotr0py <2037008807@qq.com>
As arsed in issue #12311,
When running vLLM with S3 path, it clones the meta files locally.
Current state has a bug when building the destination path for every file:
When the S3 path ends with "/" it does not clone the files, due to bug in the building path algorithm.
The solution is to use
os.path.join
A workaround for now is to have the path without "/" at the end
FIX #12311