From 7840b51958a7fb12cef4fbd42788f766a76bbf52 Mon Sep 17 00:00:00 2001 From: Tung Dang Date: Thu, 16 Nov 2023 06:28:41 +0100 Subject: [PATCH] add host & port comparision if both set in HttpClient and Settings param (#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 --- chromadb/__init__.py | 9 +++++++++ chromadb/test/test_client.py | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/chromadb/__init__.py b/chromadb/__init__.py index ccdf54a0032..596b5b82bc6 100644 --- a/chromadb/__init__.py +++ b/chromadb/__init__.py @@ -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. """ @@ -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 diff --git a/chromadb/test/test_client.py b/chromadb/test/test_client.py index 2a8aec2764e..f67293d8586 100644 --- a/chromadb/test/test_client.py +++ b/chromadb/test/test_client.py @@ -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 @@ -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]" + )