-
Notifications
You must be signed in to change notification settings - Fork 13.9k
refactor : use common download in tools/run #17535
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
Conversation
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
|
With this change, we can deprecate the cURL dependency and start shipping releases without it. |
|
My reviews are somewhat irrelevant now, I don't have merge rights, took a quick skim didn't read every line, at a glance everything seems reasonably ok, recommend doing a quick test via: llama-server -dr gemma3 to be sure... |
Here some runs: via docker (resume works) via HF: and for the output is excessively verbose, and the progress bar is broken when doing multiple downloads. However, it will be easier to improve the code from now |
|
Looks like |
Yes I’m going to check all the red alerts. 😬 |
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
|
@angt I don't know if you are interested in this but, it would be preferable if llama-run ran a client and a llama-server in parallel, llama-run/llama-server would benefit from more re-usable code |
I tried to preserve |
A CVE in linenoise ended up scratching my itch, but I'd appreciate if you built this branch, gave the new llama-run experience a shot and provided feedback for future PRs: |
|
You might want to abandon run.cpp changes here (I don't know if the other parts should stay in this PR). The new PR is a much better experience and CVE free (with the removal of linenoise) |
|
@ericcurtin, this PR still resolves some issues with |
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
|
I've noticed a pattern where my PRs are often asked to wait for your changes to merge first. While I understand the need to avoid conflicts, constantly deferring my work can be discouraging. Could we try prioritizing based on readiness this time? I have no further changes to make to my pull request at this time: In this case there isn't any significant conflict, the above PR makes no changes to download code. |
|
I apologize if you felt that way, but I must point out that the PR you mentioned happened before yours. 😅 Anyway, I was only suggesting we merge this one with yours, but more importantly, that they don’t solve the same problem. |
tools/run(removing duplicated code).cpp-httplib).