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

5663 ollama client host #5674

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rylativity
Copy link

@rylativity rylativity commented Feb 23, 2025

@ekzhu should likely be assigned as reviewer

Why are these changes needed?

These changes address the bug reported in #5663. Prevents TypeError from being thrown at inference time by ollama AsyncClient when host (and other) kwargs are passed to autogen OllamaChatCompletionClient constructor.

It also adds ollama as a named optional extra so that the ollama requirements can be installed alongside autogen-ext (e.g. pip install autogen-ext[ollama]

@ekzhu, I will need some help or guidance to ensure that the associated test (which requires ollama and tiktoken as dependencies of the OllamaChatCompletionClient) can run successfully in autogen's test execution environment.

I have also left the "I've made sure all auto checks have passed" check below unchecked as this PR is coming from my fork. (UPDATE: auto checks appear to have passed after opening PR, so I have checked box below)

Related issue number

Intended to close #5663

Checks

Ryan Stewart added 3 commits February 23, 2025 12:45
…querytime when passing host (and other) kwargs to the OllamaChatCompletionClient constructor
…y kwargs are not passed through to the ollama AsyncClient's .chat() method
…ich will install ollama client and tiktoken required by autogen-ext/src/autogen_ext/models/ollama/_ollama_client.py
@ekzhu ekzhu requested a review from peterychang February 23, 2025 18:05
@ekzhu
Copy link
Collaborator

ekzhu commented Feb 23, 2025

@rylativity you need to agree to the contributor license for this to be merged.

@rylativity
Copy link
Author

rylativity commented Feb 23, 2025 via email

@rylativity
Copy link
Author

@ekzhu I have agreed to the contributor license.

I called this out in the PR comment, but since the tests will require ollama and tiktoken (as dependencies of the OllamaChatCompletionClient), will you take a look to help me ensure that the environment where the tests will be run include those dependencies? Or give me some guidance if there are changes that need to be made?

@ekzhu
Copy link
Collaborator

ekzhu commented Feb 23, 2025

We currently don't have an environment to run ollama integration tests.

You can see this example for OpenAI client to see how to run tests locally:

@pytest.mark.asyncio
async def test_ollama() -> None:
model = "deepseek-r1:1.5b"
model_info: ModelInfo = {
"function_calling": False,
"json_output": False,
"vision": False,
"family": ModelFamily.R1,
}
# Check if the model is running locally.
try:
async with httpx.AsyncClient() as client:
response = await client.get(f"http://localhost:11434/v1/models/{model}")
response.raise_for_status()
except httpx.HTTPStatusError as e:
pytest.skip(f"{model} model is not running locally: {e}")
except httpx.ConnectError as e:
pytest.skip(f"Ollama is not running locally: {e}")
model_client = OpenAIChatCompletionClient(
model=model,
api_key="placeholder",
base_url="http://localhost:11434/v1",
model_info=model_info,
)

For unit tests, we rely on mocks.

@peterychang can you help with reviewing this PR? I think it falls into the TODOs of Ollama client.

…_kwarg to use monkeypatched ollama AsyncClient to mock httpx request to ollama server
@rylativity
Copy link
Author

@ekzhu ok thanks, I've updated the test to use a monkeypatched ollama AsyncClient with a mocked httpx call to ollama server

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants