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

Add URL path to client URL in in Client._parse_host() #170

Merged
merged 6 commits into from
Aug 23, 2024

Conversation

bplunkert
Copy link
Contributor

@bplunkert bplunkert commented May 29, 2024

This fix for #113 allows instantiating a client with a custom URL path as part of the host argument.

Resolves #244
Resolves #113

@HorseCheng
Copy link

HorseCheng commented Aug 7, 2024

Someone please approve the PR and merge the code. 🥺

ChatOllama uses this library, which has an issue that needs to be addressed as well.
The sample code below shows that the URL path "copilot" is not passed in:

from langchain_ollama import ChatOllama

chat_model = ChatOllama(
    model="llama3.1",
    base_url = "https://aaaaaaaaaaa.com/copilot"
)

chat_model.invoke("hello")

@jason-heo
Copy link

Resolves #244 as well.

I'd love to see this PR get merged.

Copy link
Collaborator

@mxyng mxyng left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I can merge this is you add some doc tests. Something like this should be sufficient:

  >>> _parse_host('example.com/path')
  'http://example.com:11434/path'
  >>> _parse_host('example.com:56789/path')
  'http://example.com:56789/path'
  >>> _parse_host('https://example.com:56789/path')
  'https://example.com:56789/path'
  >>> _parse_host('example.com:56789/path/')
  'http://example.com:56789/path'

ollama/_client.py Outdated Show resolved Hide resolved
@bplunkert
Copy link
Contributor Author

thanks @mxyng I've added your suggestions to the pull request.

ollama/_client.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@mxyng mxyng left a comment

Choose a reason for hiding this comment

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

Awesome thanks!

ollama/_client.py Outdated Show resolved Hide resolved
Co-authored-by: Michael Yang <mxyng@pm.me>
@mxyng mxyng merged commit dfdeb7c into ollama:main Aug 23, 2024
5 checks passed
@bplunkert bplunkert deleted the add_url_path branch August 28, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants