From 26c15387f31cc85a922b09c639b3a9808a9244b3 Mon Sep 17 00:00:00 2001 From: Trayan Azarov Date: Mon, 11 Dec 2023 20:22:58 +0200 Subject: [PATCH 1/9] feat: Added Auth and version mismatch error handling. --- chromadb/api/client.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/chromadb/api/client.py b/chromadb/api/client.py index 36954a65b0..250e07b231 100644 --- a/chromadb/api/client.py +++ b/chromadb/api/client.py @@ -430,6 +430,20 @@ def _validate_tenant_database(self, tenant: str, database: str) -> None: raise ValueError( "Could not connect to a Chroma server. Are you sure it is running?" ) + except requests.exceptions.HTTPError as ex: + if ex.response.status_code in [401, 403]: + raise ValueError( + "Authentication error. Have you configured your client to use authentication?" + ) + if ex.response.status_code == 404: + raise ValueError( + f"It appears your Chroma server is running an older version of Chroma {self.get_version()}. " + "Please upgrade to the latest version." + ) + else: + raise ValueError( + f"Could not connect to tenant {tenant}. Are you sure it exists?" + ) # Propagate ChromaErrors except ChromaError as e: raise e From ab652c333a30379413f2a8430d56414201e2b9a9 Mon Sep 17 00:00:00 2001 From: Trayan Azarov Date: Tue, 12 Dec 2023 12:43:39 +0200 Subject: [PATCH 2/9] feat: Improved client/server version message mismatch Refs: #1480 --- chromadb/__init__.py | 1 + chromadb/api/client.py | 16 +++++++++++----- chromadb/api/fastapi.py | 5 +---- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/chromadb/__init__.py b/chromadb/__init__.py index 522371aa17..6d928d4217 100644 --- a/chromadb/__init__.py +++ b/chromadb/__init__.py @@ -37,6 +37,7 @@ "UpdateCollectionMetadata", "QueryResult", "GetResult", + "__version__", ] logger = logging.getLogger(__name__) diff --git a/chromadb/api/client.py b/chromadb/api/client.py index 250e07b231..bb4ccee402 100644 --- a/chromadb/api/client.py +++ b/chromadb/api/client.py @@ -4,6 +4,8 @@ from overrides import override import requests + +from chromadb import __version__ as local_chroma_version from chromadb.api import AdminAPI, ClientAPI, ServerAPI from chromadb.api.types import ( CollectionMetadata, @@ -141,10 +143,9 @@ def __init__( self.database = database # Create an admin client for verifying that databases and tenants exist self._admin_client = AdminClient.from_system(self._system) - self._validate_tenant_database(tenant=tenant, database=database) - # Get the root system component we want to interact with self._server = self._system.instance(ServerAPI) + self._validate_tenant_database(tenant=tenant, database=database) # Submit event for a client start telemetry_client = self._system.instance(ProductTelemetryClient) @@ -425,7 +426,11 @@ def set_database(self, database: str) -> None: def _validate_tenant_database(self, tenant: str, database: str) -> None: try: - self._admin_client.get_tenant(name=tenant) + try: + self._admin_client.get_tenant(name=tenant) + except Exception as e1: + print(type(e1)) + raise e1 except requests.exceptions.ConnectionError: raise ValueError( "Could not connect to a Chroma server. Are you sure it is running?" @@ -437,8 +442,9 @@ def _validate_tenant_database(self, tenant: str, database: str) -> None: ) if ex.response.status_code == 404: raise ValueError( - f"It appears your Chroma server is running an older version of Chroma {self.get_version()}. " - "Please upgrade to the latest version." + f"It appears you are using newer version of Chroma client (v{local_chroma_version}) " + f"that is not compatible with Chroma server (v{self.get_version()}). " + "Please upgrade your server to the latest version." ) else: raise ValueError( diff --git a/chromadb/api/fastapi.py b/chromadb/api/fastapi.py index 2a718e4259..6033c846db 100644 --- a/chromadb/api/fastapi.py +++ b/chromadb/api/fastapi.py @@ -624,7 +624,4 @@ def raise_chroma_error(resp: requests.Response) -> None: if chroma_error: raise chroma_error - try: - resp.raise_for_status() - except requests.HTTPError: - raise (Exception(resp.text)) + resp.raise_for_status() From 772f10d07fa0cf3678e98e3b4ab74c3919d8a6fc Mon Sep 17 00:00:00 2001 From: Trayan Azarov Date: Fri, 15 Dec 2023 23:27:08 +0200 Subject: [PATCH 3/9] feat: Improvements to client/server error handling - Fixed an issue with generic JSON errors - they need to match Chroma format with Error type and error message - Cleaned up collection add unnecessary try/except - Fixed unit mock tests to pass Refs: #1480 --- chromadb/api/client.py | 113 ++++++++++++++---- chromadb/api/fastapi.py | 3 + chromadb/errors.py | 25 ++++ chromadb/server/fastapi/__init__.py | 26 ++-- .../test/client/test_client_compatibility.py | 96 +++++++++++++++ chromadb/utils/client_utils.py | 19 +++ requirements_dev.txt | 1 + 7 files changed, 244 insertions(+), 39 deletions(-) create mode 100644 chromadb/test/client/test_client_compatibility.py create mode 100644 chromadb/utils/client_utils.py diff --git a/chromadb/api/client.py b/chromadb/api/client.py index d2387668e8..ba76fcc003 100644 --- a/chromadb/api/client.py +++ b/chromadb/api/client.py @@ -1,3 +1,4 @@ +import logging from typing import ClassVar, Dict, Optional, Sequence from uuid import UUID import uuid @@ -5,8 +6,9 @@ from overrides import override import requests -from chromadb import __version__ as local_chroma_version +from chromadb import errors from chromadb.api import AdminAPI, ClientAPI, ServerAPI +from chromadb.api.fastapi import FastAPI from chromadb.api.types import ( CollectionMetadata, DataLoader, @@ -30,6 +32,9 @@ from chromadb.telemetry.product.events import ClientStartEvent from chromadb.types import Database, Tenant, Where, WhereDocument import chromadb.utils.embedding_functions as ef +from chromadb.utils.client_utils import compare_versions + +logger = logging.getLogger(__name__) class SharedSystemClient: @@ -139,13 +144,13 @@ def __init__( settings: Settings = Settings(), ) -> None: super().__init__(settings=settings) + self._validated = False self.tenant = tenant self.database = database # Create an admin client for verifying that databases and tenants exist self._admin_client = AdminClient.from_system(self._system) # Get the root system component we want to interact with self._server = self._system.instance(ServerAPI) - self._validate_tenant_database(tenant=tenant, database=database) # Submit event for a client start telemetry_client = self._system.instance(ProductTelemetryClient) @@ -169,19 +174,19 @@ def from_system( # Note - we could do this in less verbose ways, but they break type checking @override def heartbeat(self) -> int: - return self._server.heartbeat() + return self._validate()._server.heartbeat() @override def list_collections( self, limit: Optional[int] = None, offset: Optional[int] = None ) -> Sequence[Collection]: - return self._server.list_collections( + return self._validate()._server.list_collections( limit, offset, tenant=self.tenant, database=self.database ) @override def count_collections(self) -> int: - return self._server.count_collections( + return self._validate()._server.count_collections( tenant=self.tenant, database=self.database ) @@ -196,7 +201,7 @@ def create_collection( data_loader: Optional[DataLoader[Loadable]] = None, get_or_create: bool = False, ) -> Collection: - return self._server.create_collection( + return self._validate()._server.create_collection( name=name, metadata=metadata, embedding_function=embedding_function, @@ -216,7 +221,7 @@ def get_collection( ] = ef.DefaultEmbeddingFunction(), # type: ignore data_loader: Optional[DataLoader[Loadable]] = None, ) -> Collection: - return self._server.get_collection( + return self._validate()._server.get_collection( id=id, name=name, embedding_function=embedding_function, @@ -235,7 +240,7 @@ def get_or_create_collection( ] = ef.DefaultEmbeddingFunction(), # type: ignore data_loader: Optional[DataLoader[Loadable]] = None, ) -> Collection: - return self._server.get_or_create_collection( + return self._validate()._server.get_or_create_collection( name=name, metadata=metadata, embedding_function=embedding_function, @@ -251,7 +256,7 @@ def _modify( new_name: Optional[str] = None, new_metadata: Optional[CollectionMetadata] = None, ) -> None: - return self._server._modify( + return self._validate()._server._modify( id=id, new_name=new_name, new_metadata=new_metadata, @@ -262,7 +267,7 @@ def delete_collection( self, name: str, ) -> None: - return self._server.delete_collection( + return self._validate()._server.delete_collection( name=name, tenant=self.tenant, database=self.database, @@ -282,7 +287,7 @@ def _add( documents: Optional[Documents] = None, uris: Optional[URIs] = None, ) -> bool: - return self._server._add( + return self._validate()._server._add( ids=ids, collection_id=collection_id, embeddings=embeddings, @@ -301,7 +306,7 @@ def _update( documents: Optional[Documents] = None, uris: Optional[URIs] = None, ) -> bool: - return self._server._update( + return self._validate()._server._update( collection_id=collection_id, ids=ids, embeddings=embeddings, @@ -320,7 +325,7 @@ def _upsert( documents: Optional[Documents] = None, uris: Optional[URIs] = None, ) -> bool: - return self._server._upsert( + return self._validate()._server._upsert( collection_id=collection_id, ids=ids, embeddings=embeddings, @@ -331,13 +336,13 @@ def _upsert( @override def _count(self, collection_id: UUID) -> int: - return self._server._count( + return self._validate()._server._count( collection_id=collection_id, ) @override def _peek(self, collection_id: UUID, n: int = 10) -> GetResult: - return self._server._peek( + return self._validate()._server._peek( collection_id=collection_id, n=n, ) @@ -356,7 +361,7 @@ def _get( where_document: Optional[WhereDocument] = {}, include: Include = ["embeddings", "metadatas", "documents"], ) -> GetResult: - return self._server._get( + return self._validate()._server._get( collection_id=collection_id, ids=ids, where=where, @@ -376,7 +381,7 @@ def _delete( where: Optional[Where] = {}, where_document: Optional[WhereDocument] = {}, ) -> IDs: - return self._server._delete( + return self._validate()._server._delete( collection_id=collection_id, ids=ids, where=where, @@ -393,7 +398,7 @@ def _query( where_document: WhereDocument = {}, include: Include = ["embeddings", "metadatas", "documents", "distances"], ) -> QueryResult: - return self._server._query( + return self._validate()._server._query( collection_id=collection_id, query_embeddings=query_embeddings, n_results=n_results, @@ -434,23 +439,81 @@ def set_database(self, database: str) -> None: self._validate_tenant_database(tenant=self.tenant, database=database) self.database = database + def _validate_connectivity(self) -> None: + """Validates connectivity to the server. Returns True if successful, False otherwise.""" + if not isinstance(self._server, FastAPI): + return + try: + self._server.heartbeat() + except requests.exceptions.ConnectionError as e: + raise errors.GenericError( + code=-1, message=f"Chroma server seems inaccessible: {str(e)}" + ) + except requests.exceptions.HTTPError as ex: + if ex.response and ex.response.status_code in [504, 502, 503]: + # proxy errors, Gateway timeout = 504, Bad Gateway = 502, Service Unavailable = 503 + raise errors.GenericError( + code=ex.response.status_code, + message=f"Your proxy reports Chroma server might not be accessible: {str(ex)}", + ) + if ( + ex.response and ex.response.status_code == 429 + ): # do we need to handle this? + raise errors.GenericError( + code=ex.response.status_code, + message=f"Chroma server is rate limiting your requests: {str(ex)}", + ) + else: + raise errors.GenericError( + code=ex.response.status_code if ex.response else -1, message=str(ex) + ) + + def _validate(self) -> "Client": + if self._validated: + return self + self._validate_connectivity() + self._version_compatibility_check() + self._validate_tenant_database(tenant=self.tenant, database=self.database) + self._validated = True + return self + + @staticmethod + def _min_server_compatible_version() -> str: + # TODO - this should be automatically generated upon release + return "0.4.15" + + def _version_compatibility_check(self) -> None: + """Checks if the client and server versions are compatible. Raises an error if not.""" + if isinstance(self._server, FastAPI): + server_version = self.get_version() + if ( + compare_versions(server_version, self._min_server_compatible_version()) + < 0 + ): + from chromadb import __version__ as local_chroma_version + + raise ValueError( + f"It appears you are using newer version of Chroma client (v{local_chroma_version}) " + f"that is not compatible with Chroma server (v{server_version}). " + f"Please upgrade your server to a compatible version " + f"(min: v{self._min_server_compatible_version()})." + ) + def _validate_tenant_database(self, tenant: str, database: str) -> None: try: - try: - self._admin_client.get_tenant(name=tenant) - except Exception as e1: - print(type(e1)) - raise e1 + self._admin_client.get_tenant(name=tenant) except requests.exceptions.ConnectionError: raise ValueError( "Could not connect to a Chroma server. Are you sure it is running?" ) except requests.exceptions.HTTPError as ex: - if ex.response.status_code in [401, 403]: + if ex.response and ex.response.status_code in [401, 403]: raise ValueError( "Authentication error. Have you configured your client to use authentication?" ) - if ex.response.status_code == 404: + if ex.response and ex.response.status_code == 404: + from chromadb import __version__ as local_chroma_version + raise ValueError( f"It appears you are using newer version of Chroma client (v{local_chroma_version}) " f"that is not compatible with Chroma server (v{self.get_version()}). " diff --git a/chromadb/api/fastapi.py b/chromadb/api/fastapi.py index 2edb8a9b2b..21bdd74c28 100644 --- a/chromadb/api/fastapi.py +++ b/chromadb/api/fastapi.py @@ -639,7 +639,10 @@ def raise_chroma_error(resp: requests.Response) -> None: if "error" in body: if body["error"] in errors.error_types: chroma_error = errors.error_types[body["error"]](body["message"]) + else: + chroma_error = errors.GenericError(resp.status_code, body["message"]) + # TODO do we need to catch BaseException here? except BaseException: pass diff --git a/chromadb/errors.py b/chromadb/errors.py index faf441d870..132c0013eb 100644 --- a/chromadb/errors.py +++ b/chromadb/errors.py @@ -82,6 +82,31 @@ def name(cls) -> str: return "AuthorizationError" +class GenericError(ChromaError): + def __init__(self, code: int, message: str) -> None: + self._code = code + self._message = message + + @overrides + def code(self) -> int: + return self._code + + @overrides + def message(self) -> str: + return self._message + + @classmethod + @overrides + def name(cls) -> str: + return "ServerError" + + def __str__(self) -> str: + return f"{self.name()}(code={self._code}, message={self._message})" + + def __repr__(self) -> str: + return str(self) + + error_types: Dict[str, Type[ChromaError]] = { "InvalidDimension": InvalidDimensionException, "InvalidCollection": InvalidCollectionException, diff --git a/chromadb/server/fastapi/__init__.py b/chromadb/server/fastapi/__init__.py index f913bc6aa5..d87aa8090b 100644 --- a/chromadb/server/fastapi/__init__.py +++ b/chromadb/server/fastapi/__init__.py @@ -5,7 +5,7 @@ from fastapi.middleware.cors import CORSMiddleware from fastapi.routing import APIRoute -from fastapi import HTTPException, status +from fastapi import status from uuid import UUID import chromadb @@ -36,7 +36,6 @@ from chromadb.errors import ( ChromaError, InvalidUUIDError, - InvalidDimensionException, InvalidHTTPVersion, ) from chromadb.server.fastapi.types import ( @@ -85,7 +84,9 @@ async def catch_exceptions_middleware( return e.fastapi_json_response() except Exception as e: logger.exception(e) - return JSONResponse(content={"error": repr(e)}, status_code=500) + return JSONResponse( + content={"error": str(type(e)), "message": f"e{str(e)}"}, status_code=500 + ) async def check_http_version_middleware( @@ -486,17 +487,14 @@ def delete_collection( ), ) def add(self, collection_id: str, add: AddEmbedding) -> None: - try: - result = self._api._add( - collection_id=_uuid(collection_id), - embeddings=add.embeddings, # type: ignore - metadatas=add.metadatas, # type: ignore - documents=add.documents, # type: ignore - uris=add.uris, # type: ignore - ids=add.ids, - ) - except InvalidDimensionException as e: - raise HTTPException(status_code=500, detail=str(e)) + result = self._api._add( + collection_id=_uuid(collection_id), + embeddings=add.embeddings, # type: ignore + metadatas=add.metadatas, # type: ignore + documents=add.documents, # type: ignore + uris=add.uris, # type: ignore + ids=add.ids, + ) return result # type: ignore @trace_method("FastAPI.update", OpenTelemetryGranularity.OPERATION) diff --git a/chromadb/test/client/test_client_compatibility.py b/chromadb/test/client/test_client_compatibility.py new file mode 100644 index 0000000000..6d9567189f --- /dev/null +++ b/chromadb/test/client/test_client_compatibility.py @@ -0,0 +1,96 @@ +import json +import uuid +import time + +import pytest +from hypothesis import given, strategies as st +from pytest_httpserver import HTTPServer + +import chromadb +from chromadb.errors import GenericError +from chromadb.types import Tenant, Database + + +def test_incompatible_server_version(caplog: pytest.LogCaptureFixture) -> None: + with HTTPServer(port=8001) as httpserver: + httpserver.expect_request("/api/v1/collections").respond_with_data( + json.dumps([]) + ) + httpserver.expect_request("/api/v1/heartbeat").respond_with_data( + json.dumps({"nanosecond heartbeat": int(time.time_ns())}) + ) + + httpserver.expect_request("/api/v1").respond_with_data( + json.dumps({"nanosecond heartbeat": int(time.time_ns())}) + ) + + httpserver.expect_request("/api/v1/version").respond_with_data( + json.dumps("0.4.1") + ) + client = chromadb.HttpClient("http://localhost:8001") + + with pytest.raises(ValueError) as e: + client.list_collections() + assert "It appears you are using newer version of Chroma client" in str(e.value) + + +def test_compatible_server_version(caplog: pytest.LogCaptureFixture) -> None: + with HTTPServer(port=8001) as httpserver: + httpserver.expect_request("/api/v1/collections").respond_with_data( + json.dumps([]) + ) + httpserver.expect_request("/api/v1/heartbeat").respond_with_data( + json.dumps({"nanosecond heartbeat": int(time.time_ns())}) + ) + + httpserver.expect_request("/api/v1").respond_with_data( + json.dumps({"nanosecond heartbeat": int(time.time_ns())}) + ) + + httpserver.expect_request("/api/v1/version").respond_with_data( + json.dumps("0.4.15") + ) + httpserver.expect_request("/api/v1/tenants/default_tenant").respond_with_data( + json.dumps(Tenant(name="default_tenant")) + ) + httpserver.expect_request( + "/api/v1/databases/default_database" + ).respond_with_data( + json.dumps( + Database( + name="default_database", + tenant="default_tenant", + id=str(uuid.uuid4()), # type: ignore + ) + ) + ) + + client = chromadb.HttpClient("http://localhost:8001") + + client.list_collections() + + +def test_client_server_not_available(caplog: pytest.LogCaptureFixture) -> None: + with HTTPServer(port=8002) as _: + client = chromadb.HttpClient("http://localhost:8001") + + with pytest.raises(GenericError) as e: + client.list_collections() + assert "Chroma server seems inaccessible" in str(e.value) + + +@given(status=st.sampled_from([502, 503, 504])) +def test_client_server_with_proxy_error( + status: int, caplog: pytest.LogCaptureFixture +) -> None: + with HTTPServer(port=8001) as httpserver: + httpserver.expect_request("/api/v1/heartbeat").respond_with_data( + "Oh no!", status=status + ) + + httpserver.expect_request("/api/v1").respond_with_data("Oh no!", status=status) + client = chromadb.HttpClient("http://localhost:8001") + + with pytest.raises(GenericError) as e: + client.list_collections() + assert "Your proxy reports Chroma server might not be" in str(e.value) diff --git a/chromadb/utils/client_utils.py b/chromadb/utils/client_utils.py new file mode 100644 index 0000000000..bfbb1d9361 --- /dev/null +++ b/chromadb/utils/client_utils.py @@ -0,0 +1,19 @@ +def compare_versions(version1: str, version2: str) -> int: + """Compares two versions of the format X.Y.Z and returns 1 if version1 is greater than version2, -1 if version1 is + less than version2, and 0 if version1 is equal to version2. + """ + v1_components = list(map(int, version1.split("."))) + v2_components = list(map(int, version2.split("."))) + + for v1, v2 in zip(v1_components, v2_components): + if v1 > v2: + return 1 + elif v1 < v2: + return -1 + + if len(v1_components) > len(v2_components): + return 1 + elif len(v1_components) < len(v2_components): + return -1 + + return 0 diff --git a/requirements_dev.txt b/requirements_dev.txt index 4dce86e2ef..90696e499b 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -8,6 +8,7 @@ mypy-protobuf pre-commit pytest pytest-asyncio +pytest-httpserver setuptools_scm types-protobuf types-requests==2.30.0.0 From d8c64dc9c4d081ece95fc1d2f38bf4f5fd88a79e Mon Sep 17 00:00:00 2001 From: Trayan Azarov Date: Fri, 15 Dec 2023 23:36:43 +0200 Subject: [PATCH 4/9] fix: Fixed failing test Refs: #1480 --- chromadb/api/client.py | 19 +++++-------------- .../test/client/test_client_compatibility.py | 1 + 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/chromadb/api/client.py b/chromadb/api/client.py index ba76fcc003..45761f57b5 100644 --- a/chromadb/api/client.py +++ b/chromadb/api/client.py @@ -450,23 +450,14 @@ def _validate_connectivity(self) -> None: code=-1, message=f"Chroma server seems inaccessible: {str(e)}" ) except requests.exceptions.HTTPError as ex: - if ex.response and ex.response.status_code in [504, 502, 503]: + if ex.response.status_code in [504, 502, 503]: # type: ignore # proxy errors, Gateway timeout = 504, Bad Gateway = 502, Service Unavailable = 503 raise errors.GenericError( - code=ex.response.status_code, + code=ex.response.status_code, # type: ignore message=f"Your proxy reports Chroma server might not be accessible: {str(ex)}", ) - if ( - ex.response and ex.response.status_code == 429 - ): # do we need to handle this? - raise errors.GenericError( - code=ex.response.status_code, - message=f"Chroma server is rate limiting your requests: {str(ex)}", - ) else: - raise errors.GenericError( - code=ex.response.status_code if ex.response else -1, message=str(ex) - ) + raise errors.GenericError(code=ex.response.status_code, message=str(ex)) # type: ignore def _validate(self) -> "Client": if self._validated: @@ -507,11 +498,11 @@ def _validate_tenant_database(self, tenant: str, database: str) -> None: "Could not connect to a Chroma server. Are you sure it is running?" ) except requests.exceptions.HTTPError as ex: - if ex.response and ex.response.status_code in [401, 403]: + if ex.response.status_code in [401, 403]: # type: ignore raise ValueError( "Authentication error. Have you configured your client to use authentication?" ) - if ex.response and ex.response.status_code == 404: + if ex.response.status_code == 404: # type: ignore from chromadb import __version__ as local_chroma_version raise ValueError( diff --git a/chromadb/test/client/test_client_compatibility.py b/chromadb/test/client/test_client_compatibility.py index 6d9567189f..4b22ec3ce0 100644 --- a/chromadb/test/client/test_client_compatibility.py +++ b/chromadb/test/client/test_client_compatibility.py @@ -93,4 +93,5 @@ def test_client_server_with_proxy_error( with pytest.raises(GenericError) as e: client.list_collections() + print(str(e.value)) assert "Your proxy reports Chroma server might not be" in str(e.value) From 0a3bf3f22c681f5465f6b470fd859c9cf29c64d0 Mon Sep 17 00:00:00 2001 From: Trayan Azarov Date: Mon, 18 Dec 2023 11:04:30 +0200 Subject: [PATCH 5/9] fix: Fixing failing JS test TODO - regen of the API is needed for error message. Refs: #1480 --- clients/js/test/client.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clients/js/test/client.test.ts b/clients/js/test/client.test.ts index 512237a245..85270ec7e0 100644 --- a/clients/js/test/client.test.ts +++ b/clients/js/test/client.test.ts @@ -191,5 +191,6 @@ test('wrong code returns an error', async () => { // @ts-ignore - supposed to fail const results = await collection.get({ where: { "test": { "$contains": "hello" } } }); expect(results.error).toBeDefined() - expect(results.error).toContain("ValueError('Expected where operator") + console.log(results) + expect(results.error).toContain("ValueError") }) From 9da597658f3ce8f574c2975a292bab003f55d761 Mon Sep 17 00:00:00 2001 From: Trayan Azarov Date: Mon, 18 Dec 2023 17:52:12 +0200 Subject: [PATCH 6/9] fix: Adding reset client settings to client compatibility tests Refs: #1480 --- chromadb/test/client/test_client_compatibility.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/chromadb/test/client/test_client_compatibility.py b/chromadb/test/client/test_client_compatibility.py index 4b22ec3ce0..00438bb01e 100644 --- a/chromadb/test/client/test_client_compatibility.py +++ b/chromadb/test/client/test_client_compatibility.py @@ -7,10 +7,16 @@ from pytest_httpserver import HTTPServer import chromadb +from chromadb.api.client import SharedSystemClient from chromadb.errors import GenericError from chromadb.types import Tenant, Database +@pytest.fixture(autouse=True) +def reset_client_settings() -> None: + SharedSystemClient.clear_system_cache() + + def test_incompatible_server_version(caplog: pytest.LogCaptureFixture) -> None: with HTTPServer(port=8001) as httpserver: httpserver.expect_request("/api/v1/collections").respond_with_data( From 95ade4c97bf06b73659046e83d22082c8df2820f Mon Sep 17 00:00:00 2001 From: Trayan Azarov Date: Mon, 18 Dec 2023 20:37:00 +0200 Subject: [PATCH 7/9] fix: Adding port/host to clients in compat. test Refs: #1480 --- chromadb/test/client/test_client_compatibility.py | 9 ++++----- clients/js/test/client.test.ts | 1 - 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/chromadb/test/client/test_client_compatibility.py b/chromadb/test/client/test_client_compatibility.py index 00438bb01e..5d473d4bfe 100644 --- a/chromadb/test/client/test_client_compatibility.py +++ b/chromadb/test/client/test_client_compatibility.py @@ -33,7 +33,7 @@ def test_incompatible_server_version(caplog: pytest.LogCaptureFixture) -> None: httpserver.expect_request("/api/v1/version").respond_with_data( json.dumps("0.4.1") ) - client = chromadb.HttpClient("http://localhost:8001") + client = chromadb.HttpClient(host="localhost", port="8001") with pytest.raises(ValueError) as e: client.list_collections() @@ -71,14 +71,14 @@ def test_compatible_server_version(caplog: pytest.LogCaptureFixture) -> None: ) ) - client = chromadb.HttpClient("http://localhost:8001") + client = chromadb.HttpClient(host="localhost", port="8001") client.list_collections() def test_client_server_not_available(caplog: pytest.LogCaptureFixture) -> None: with HTTPServer(port=8002) as _: - client = chromadb.HttpClient("http://localhost:8001") + client = chromadb.HttpClient(host="localhost", port="8001") with pytest.raises(GenericError) as e: client.list_collections() @@ -95,9 +95,8 @@ def test_client_server_with_proxy_error( ) httpserver.expect_request("/api/v1").respond_with_data("Oh no!", status=status) - client = chromadb.HttpClient("http://localhost:8001") + client = chromadb.HttpClient(host="localhost", port="8001") with pytest.raises(GenericError) as e: client.list_collections() - print(str(e.value)) assert "Your proxy reports Chroma server might not be" in str(e.value) diff --git a/clients/js/test/client.test.ts b/clients/js/test/client.test.ts index 85270ec7e0..56af0ec501 100644 --- a/clients/js/test/client.test.ts +++ b/clients/js/test/client.test.ts @@ -191,6 +191,5 @@ test('wrong code returns an error', async () => { // @ts-ignore - supposed to fail const results = await collection.get({ where: { "test": { "$contains": "hello" } } }); expect(results.error).toBeDefined() - console.log(results) expect(results.error).toContain("ValueError") }) From db1daf6268a5c0b9bd13beb22a0c9fead5343f1e Mon Sep 17 00:00:00 2001 From: Trayan Azarov Date: Mon, 18 Dec 2023 21:28:28 +0200 Subject: [PATCH 8/9] fix: Changing the way client info is cleared Refs: #1480 --- chromadb/test/client/test_client_compatibility.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chromadb/test/client/test_client_compatibility.py b/chromadb/test/client/test_client_compatibility.py index 5d473d4bfe..c64eb55eac 100644 --- a/chromadb/test/client/test_client_compatibility.py +++ b/chromadb/test/client/test_client_compatibility.py @@ -14,7 +14,7 @@ @pytest.fixture(autouse=True) def reset_client_settings() -> None: - SharedSystemClient.clear_system_cache() + SharedSystemClient._identifer_to_system = {} def test_incompatible_server_version(caplog: pytest.LogCaptureFixture) -> None: From 6bd142c9982d7edfc3cb359c0c363141a1da8442 Mon Sep 17 00:00:00 2001 From: Trayan Azarov Date: Tue, 19 Dec 2023 09:52:03 +0200 Subject: [PATCH 9/9] fix: Fixed unit tests - Exported Settings in __all__ Refs: #1480 --- chromadb/__init__.py | 1 + .../test/client/test_client_compatibility.py | 26 +++++++++++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/chromadb/__init__.py b/chromadb/__init__.py index eebed82aea..d7a52fd9a6 100644 --- a/chromadb/__init__.py +++ b/chromadb/__init__.py @@ -38,6 +38,7 @@ "QueryResult", "GetResult", "__version__", + "Settings", ] logger = logging.getLogger(__name__) diff --git a/chromadb/test/client/test_client_compatibility.py b/chromadb/test/client/test_client_compatibility.py index c64eb55eac..a4d65f9fe8 100644 --- a/chromadb/test/client/test_client_compatibility.py +++ b/chromadb/test/client/test_client_compatibility.py @@ -14,7 +14,7 @@ @pytest.fixture(autouse=True) def reset_client_settings() -> None: - SharedSystemClient._identifer_to_system = {} + SharedSystemClient.clear_system_cache() def test_incompatible_server_version(caplog: pytest.LogCaptureFixture) -> None: @@ -33,7 +33,11 @@ def test_incompatible_server_version(caplog: pytest.LogCaptureFixture) -> None: httpserver.expect_request("/api/v1/version").respond_with_data( json.dumps("0.4.1") ) - client = chromadb.HttpClient(host="localhost", port="8001") + client = chromadb.HttpClient( + host="localhost", + port="8001", + settings=chromadb.Settings(chroma_server_http_port=8001), + ) with pytest.raises(ValueError) as e: client.list_collections() @@ -71,14 +75,22 @@ def test_compatible_server_version(caplog: pytest.LogCaptureFixture) -> None: ) ) - client = chromadb.HttpClient(host="localhost", port="8001") + client = chromadb.HttpClient( + host="localhost", + port="8001", + settings=chromadb.Settings(chroma_server_http_port=8001), + ) client.list_collections() def test_client_server_not_available(caplog: pytest.LogCaptureFixture) -> None: with HTTPServer(port=8002) as _: - client = chromadb.HttpClient(host="localhost", port="8001") + client = chromadb.HttpClient( + host="localhost", + port="8001", + settings=chromadb.Settings(chroma_server_http_port=8001), + ) with pytest.raises(GenericError) as e: client.list_collections() @@ -95,7 +107,11 @@ def test_client_server_with_proxy_error( ) httpserver.expect_request("/api/v1").respond_with_data("Oh no!", status=status) - client = chromadb.HttpClient(host="localhost", port="8001") + client = chromadb.HttpClient( + host="localhost", + port="8001", + settings=chromadb.Settings(chroma_server_http_port=8001), + ) with pytest.raises(GenericError) as e: client.list_collections()