Skip to content

Implement s3:// protocol #11511

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
Feb 1, 2025
Merged

Implement s3:// protocol #11511

merged 1 commit into from
Feb 1, 2025

Conversation

ericcurtin
Copy link
Collaborator

For those that want to pull from s3

@ericcurtin ericcurtin force-pushed the s3 branch 7 times, most recently from d088b6a to df55a0c Compare January 30, 2025 17:11
@ericcurtin
Copy link
Collaborator Author

This is ready for review @ngxson

Copy link
Collaborator

@ngxson ngxson left a comment

Choose a reason for hiding this comment

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

I don't know much about s3 bucket API, so I only reviewed based on code semantic for this PR.

For those that want to pull from s3

Signed-off-by: Eric Curtin <[email protected]>
@lexasub
Copy link
Contributor

lexasub commented Jan 31, 2025

@ericcurtin, Please replace the hardcoded URL with a user-defined one for custom endpoints, such as local MinIO. Also, if the model is downloaded from S3 to the local filesystem, it's acceptable but not ideal. Is it possible to load the model directly to the vram/ram from S3? (For example, you might try this in the next PR.)

@lexasub
Copy link
Contributor

lexasub commented Jan 31, 2025

@ngxson The comment I wrote points out the rigidity of relying on a predefined endpoint and critiques the approach of loading a model directly from an internet-hosted S3 bucket, arguing that this scenario is likely uncommon. Instead, I emphasized the practicality of using a locally hosted S3 instance, which seems more applicable in real-world use cases.

@ericcurtin
Copy link
Collaborator Author

@lexasub I don't actually know that much about s3:// , I'm putting in just enough to have some sort of initial implementation we can enhance and expand upon. If you opened PRs making this even better I think that would be super cool, I don't even know what MinIO is for example.

@ericcurtin
Copy link
Collaborator Author

We have discussed eventually moving this protocol stuff to more llama.cpp binaries like llama-server, just trying to mature it a bit in llama-run first.

@ericcurtin ericcurtin merged commit ecef206 into master Feb 1, 2025
44 of 45 checks passed
@ericcurtin ericcurtin deleted the s3 branch February 1, 2025 10:30
@isaac-mcfadyen
Copy link
Contributor

@lexasub I don't actually know that much about s3:// , I'm putting in just enough to have some sort of initial implementation we can enhance and expand upon. If you opened PRs making this even better I think that would be super cool, I don't even know what MinIO is for example.

I'd +1 the MinIO thing in a followup PR. MinIO is basically local (self-hosted) S3. The ability to change the endpoint would allow users to pull from their own MinIO instance (for example, from a NAS with many TB of storage).

tinglou pushed a commit to tinglou/llama.cpp that referenced this pull request Feb 13, 2025
For those that want to pull from s3

Signed-off-by: Eric Curtin <[email protected]>
orca-zhang pushed a commit to orca-zhang/llama.cpp that referenced this pull request Feb 26, 2025
For those that want to pull from s3

Signed-off-by: Eric Curtin <[email protected]>
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Feb 26, 2025
For those that want to pull from s3

Signed-off-by: Eric Curtin <[email protected]>
mglambda pushed a commit to mglambda/llama.cpp that referenced this pull request Mar 8, 2025
For those that want to pull from s3

Signed-off-by: Eric Curtin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants