Skip to content

Commit

Permalink
[BUG] Fix circular import when initializing a client with basic auth (#…
Browse files Browse the repository at this point in the history
…1775)

## Description of changes

*Summarize the changes made by this PR.*
 - Improvements & Bug fixes
	 - Fixes #1554 (again)

## Test plan
*How are these changes tested?*

- [ ] 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)?*
  • Loading branch information
beggers authored Feb 27, 2024
1 parent 6fa6186 commit d4230b3
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 7 deletions.
7 changes: 3 additions & 4 deletions chromadb/auth/fastapi.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import chromadb
from contextvars import ContextVar
from functools import wraps
import logging
Expand Down Expand Up @@ -28,7 +27,7 @@
)
from chromadb.auth.registry import resolve_provider
from chromadb.errors import AuthorizationError
from chromadb.server.fastapi.utils import fastapi_json_response
from chromadb.utils.fastapi import fastapi_json_response
from chromadb.telemetry.opentelemetry import (
OpenTelemetryGranularity,
trace_method,
Expand Down Expand Up @@ -117,7 +116,7 @@ def instrument_server(self, app: ASGIApp) -> None:
raise NotImplementedError("Not implemented yet")


class FastAPIChromaAuthMiddlewareWrapper(BaseHTTPMiddleware): # type: ignore
class FastAPIChromaAuthMiddlewareWrapper(BaseHTTPMiddleware):
def __init__(
self, app: ASGIApp, auth_middleware: FastAPIChromaAuthMiddleware
) -> None:
Expand Down Expand Up @@ -302,7 +301,7 @@ def instrument_server(self, app: ASGIApp) -> None:
raise NotImplementedError("Not implemented yet")


class FastAPIChromaAuthzMiddlewareWrapper(BaseHTTPMiddleware): # type: ignore
class FastAPIChromaAuthzMiddlewareWrapper(BaseHTTPMiddleware):
def __init__(
self, app: ASGIApp, authz_middleware: FastAPIChromaAuthzMiddleware
) -> None:
Expand Down
2 changes: 1 addition & 1 deletion chromadb/auth/fastapi_utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from functools import partial
from typing import Any, Callable, Dict, Optional, Sequence, cast
from chromadb.server.fastapi.utils import string_to_uuid
from chromadb.utils.fastapi import string_to_uuid
from chromadb.api import ServerAPI
from chromadb.auth import AuthzResourceTypes

Expand Down
2 changes: 1 addition & 1 deletion chromadb/server/fastapi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@

import logging

from chromadb.server.fastapi.utils import fastapi_json_response, string_to_uuid as _uuid
from chromadb.utils.fastapi import fastapi_json_response, string_to_uuid as _uuid
from chromadb.telemetry.opentelemetry.fastapi import instrument_fastapi
from chromadb.types import Database, Tenant
from chromadb.telemetry.product import ServerContext, ProductTelemetryClient
Expand Down
27 changes: 27 additions & 0 deletions chromadb/test/client/create_http_client_with_basic_auth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# This file is used by test_create_http_client.py to test the initialization
# of an HttpClient class with auth settings.
#
# See https://github.com/chroma-core/chroma/issues/1554

import chromadb
from chromadb.config import Settings
import sys

def main() -> None:
try:
chromadb.HttpClient(
host='localhost',
port=8000,
settings=Settings(
chroma_client_auth_provider="chromadb.auth.basic.BasicAuthClientProvider",
chroma_client_auth_credentials="admin:testDb@home2"
)
)
except ValueError:
# We don't expect to be able to connect to Chroma. We just want to make sure
# there isn't an ImportError.
sys.exit(0)


if __name__ == "__main__":
main()
15 changes: 15 additions & 0 deletions chromadb/test/client/test_create_http_client.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import subprocess

# Needs to be a module, not a file, so that local imports work.
TEST_MODULE = "chromadb.test.client.create_http_client_with_basic_auth"


def test_main() -> None:
# This is the only way to test what we want to test: pytest does a bunch of
# importing and other module stuff in the background, so we need a clean
# python process to make sure we're not circular-importing.
#
# See https://github.com/chroma-core/chroma/issues/1554

res = subprocess.run(['python', '-m', TEST_MODULE])
assert res.returncode == 0
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ def fastapi_json_response(error: ChromaError) -> JSONResponse:
status_code=error.code(),
)


def string_to_uuid(uuid_str: str) -> UUID:
try:
return UUID(uuid_str)
except ValueError:
raise InvalidUUIDError(f"Could not parse {uuid_str} as a UUID")
raise InvalidUUIDError(f"Could not parse {uuid_str} as a UUID")

0 comments on commit d4230b3

Please sign in to comment.