Skip to content

Commit

Permalink
Serve resources from API routes ending in / (#926)
Browse files Browse the repository at this point in the history
## Description of changes

#895

Serve registered API routes regardless of whether they end in "/".

In the linked bug @HammadB and I discussed looping over all the routes
at the end of `__init__`, but the typechecker complains: the only thing
we know about `self._app.routes` is that they're instances of
`BaseRoute`, which may not have the fields we need. Rather than do type
coercion I subclassed `fastapi.APIRouter`.

The logic to set `kwargs["include_in_schema"]` is maybe too clever,
should we just do an if-else instead?

## Test plan

```
0 ~ beggers % curl http://localhost:8000/api/v1/collections/ -v
*   Trying 127.0.0.1:8000...
* Connected to localhost (127.0.0.1) port 8000 (#0)
> GET /api/v1/collections/ HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/8.1.2
> Accept: */*
> 
< HTTP/1.1 200 OK
< date: Thu, 03 Aug 2023 19:14:01 GMT
< server: uvicorn
< content-length: 2
< content-type: application/json
< 
* Connection #0 to host localhost left intact
[]%                                                                                   
0 ~ beggers % curl http://localhost:8000/api/v1/collections -v 
*   Trying 127.0.0.1:8000...
* Connected to localhost (127.0.0.1) port 8000 (#0)
> GET /api/v1/collections HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/8.1.2
> Accept: */*
> 
< HTTP/1.1 200 OK
< date: Thu, 03 Aug 2023 19:14:02 GMT
< server: uvicorn
< content-length: 2
< content-type: application/json
< 
* Connection #0 to host localhost left intact
[]%
```

Also, `pytest` locally and CI. I'd like to confirm this doesn't break JS
client generation and doc generation -- how can I do that?

## 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)?*

No documentation changes needed for this.
  • Loading branch information
beggers authored Aug 4, 2023
1 parent 1538696 commit 4c4cdac
Showing 1 changed file with 28 additions and 1 deletion.
29 changes: 28 additions & 1 deletion chromadb/server/fastapi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,33 @@ def _uuid(uuid_str: str) -> UUID:
raise InvalidUUIDError(f"Could not parse {uuid_str} as a UUID")


class ChromaAPIRouter(fastapi.APIRouter):
# A simple subclass of fastapi's APIRouter which treats URLs with a trailing "/" the
# same as URLs without. Docs will only contain URLs without trailing "/"s.
def add_api_route(self, path: str, *args: Any, **kwargs: Any) -> None:
# If kwargs["include_in_schema"] isn't passed OR is True, we should only
# include the non-"/" path. If kwargs["include_in_schema"] is False, include
# neither.
exclude_from_schema = (
"include_in_schema" in kwargs and not kwargs["include_in_schema"]
)

def include_in_schema(path: str) -> bool:
nonlocal exclude_from_schema
return not exclude_from_schema and not path.endswith("/")

kwargs["include_in_schema"] = include_in_schema(path)
super().add_api_route(path, *args, **kwargs)

if path.endswith("/"):
path = path[:-1]
else:
path = path + "/"

kwargs["include_in_schema"] = include_in_schema(path)
super().add_api_route(path, *args, **kwargs)


class FastAPI(chromadb.server.Server):
def __init__(self, settings: Settings):
super().__init__(settings)
Expand All @@ -84,7 +111,7 @@ def __init__(self, settings: Settings):
allow_methods=["*"],
)

self.router = fastapi.APIRouter()
self.router = ChromaAPIRouter()

self.router.add_api_route("/api/v1", self.root, methods=["GET"])
self.router.add_api_route("/api/v1/reset", self.reset, methods=["POST"])
Expand Down

0 comments on commit 4c4cdac

Please sign in to comment.