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

Handle missing model in CLI parameters for llama-run #11399

Merged

Conversation

engelmi
Copy link
Contributor

@engelmi engelmi commented Jan 24, 2025

The HTTP client in llama-run only prints an error in case the download of a resource failed. If the model name in the CLI parameter list is missing, this causes the application to crash.
In order to prevent this, a check for the required model parameter has been added and errors for resource downloads get propagated to the caller.

This could cause llama-run to crash:

$ llama-run --jinja
curl_easy_perform() failed: HTTP response code said error
terminate called after throwing an instance of 'nlohmann::json_abi_v3_11_3::detail::parse_error'
  what():  [json.exception.parse_error.101] parse error at line 1, column 1: attempting to parse an empty input; check that your input string or stream contains the expected JSON
Aborted (core dumped)

With the model name missing, it proceeds to try and download the resource. The curl error gets printed, but isn't handled further and so the json parsing crashes the program.

Make sure to read the contributing guidelines before submitting a PR

@engelmi
Copy link
Contributor Author

engelmi commented Jan 24, 2025

@ericcurtin PTAL

The HTTP client in llama-run only prints an error in case the download of
a resource failed. If the model name in the CLI parameter list is missing,
this causes the application to crash.
In order to prevent this, a check for the required model parameter has been
added and errors for resource downloads get propagated to the caller.

Signed-off-by: Michael Engel <mengel@redhat.com>
@engelmi engelmi force-pushed the llama-run-handle-missing-model-in-cli branch from 9b0c889 to c1973cf Compare January 24, 2025 22:05
@ericcurtin ericcurtin merged commit 2b8525d into ggerganov:master Jan 28, 2025
44 checks passed
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.

2 participants