Skip to content

Commit

Permalink
add host & port comparision if both set in HttpClient and Settings pa…
Browse files Browse the repository at this point in the history
…ram (#1266)

## Description of changes

*Summarize the changes made by this PR.*
 - Improvements
- Check if HttpClient is instantiated with inconsistent server and port
values, see #1261

## Test plan
*How are these changes tested?*
- [x] add tests for different HttpClient parameter scenarios
- [x] Tests pass locally with `pytest` for python, `yarn test` for js

## Documentation Changes
*Are all docstrings for user-facing APIs updated if required? Do we need
to make documentation changes in the [docs
repository](https://github.com/chroma-core/docs)?*

docstring is updated for settings, no docs update

Co-authored-by: Hammad Bashir <[email protected]>
  • Loading branch information
3cham and HammadB authored Nov 16, 2023
1 parent 707b7d5 commit 7840b51
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
9 changes: 9 additions & 0 deletions chromadb/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ def HttpClient(
port: The port of the Chroma server. Defaults to "8000".
ssl: Whether to use SSL to connect to the Chroma server. Defaults to False.
headers: A dictionary of headers to send to the Chroma server. Defaults to {}.
settings: A dictionary of settings to communicate with the chroma server.
tenant: The tenant to use for this client. Defaults to the default tenant.
database: The database to use for this client. Defaults to the default database.
"""
Expand All @@ -164,7 +165,15 @@ def HttpClient(
settings = Settings()

settings.chroma_api_impl = "chromadb.api.fastapi.FastAPI"
if settings.chroma_server_host and settings.chroma_server_host != host:
raise ValueError(
f"Chroma server host provided in settings[{settings.chroma_server_host}] is different to the one provided in HttpClient: [{host}]"
)
settings.chroma_server_host = host
if settings.chroma_server_http_port and settings.chroma_server_http_port != port:
raise ValueError(
f"Chroma server http port provided in settings[{settings.chroma_server_http_port}] is different to the one provided in HttpClient: [{port}]"
)
settings.chroma_server_http_port = port
settings.chroma_server_ssl_enabled = ssl
settings.chroma_server_headers = headers
Expand Down
26 changes: 26 additions & 0 deletions chromadb/test/test_client.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Generator
from unittest.mock import patch
import chromadb
from chromadb.config import Settings
from chromadb.api import ClientAPI
import chromadb.server.fastapi
import pytest
Expand Down Expand Up @@ -44,3 +45,28 @@ def test_persistent_client(persistent_api: ClientAPI) -> None:
def test_http_client(http_api: ClientAPI) -> None:
settings = http_api.get_settings()
assert settings.chroma_api_impl == "chromadb.api.fastapi.FastAPI"


def test_http_client_with_inconsistent_host_settings() -> None:
try:
chromadb.HttpClient(settings=Settings(chroma_server_host="127.0.0.1"))
except ValueError as e:
assert (
str(e)
== "Chroma server host provided in settings[127.0.0.1] is different to the one provided in HttpClient: [localhost]"
)


def test_http_client_with_inconsistent_port_settings() -> None:
try:
chromadb.HttpClient(
port="8002",
settings=Settings(
chroma_server_http_port="8001",
),
)
except ValueError as e:
assert (
str(e)
== "Chroma server http port provided in settings[8001] is different to the one provided in HttpClient: [8002]"
)

0 comments on commit 7840b51

Please sign in to comment.