-
Notifications
You must be signed in to change notification settings - Fork 76
Pass down base URL and API key to completion handler #1002
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
api_key: Optional[str], | ||
stream: bool = False, | ||
is_fim_request: bool = False, | ||
) -> Union[ChatResponse, GenerateResponse]: | ||
"""Stream response directly from Ollama API.""" | ||
|
||
if not base_url: | ||
raise ValueError("base_url is required for Ollama") |
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.
could we default to localhost:11434
?
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.
We should already get this default frm the caller.
# TODO: Add CodeGate user agent. | ||
headers = dict() | ||
if api_key: | ||
headers["Authorization"] = f"Bearer {api_key}" |
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.
Does the bearer token get forwarded or does it have some other meaning later on?
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.
It gets set so we can forward it
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.
The PR needs a rebase but in general the code looks good and I tested Ollama chat in continue which still works great.
222d2f3
to
dd3db9f
Compare
In order to make Ollama work with API Keys, I needed to change the completion handler to take a base URL and also leverage a given API key (if available). Signed-off-by: Juan Antonio Osorio <[email protected]> Co-Authored-by: Alejandro Ponce de Leon <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
5b8a04e
to
d5281d2
Compare
Signed-off-by: Juan Antonio Osorio <[email protected]>
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.
I kicked the tests, that looked like a transient error
In order to make Ollama work with API Keys, I needed to change the
completion handler to take a base URL and also leverage a given API key
(if available).
Signed-off-by: Juan Antonio Osorio [email protected]
Co-Authored-by: Alejandro Ponce de Leon [email protected]