-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update fastapi types to fix ts client codegen #2406
base: main
Are you sure you want to change the base?
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
Please tag your PR title with one of: [ENH | BUG | DOC | TST | BLD | PERF | TYP | CLN | CHORE]. See https://docs.trychroma.com/contributing#contributing-code-and-ideas |
@@ -451,7 +453,7 @@ def process_create_tenant(request: Request, raw_body: bytes) -> None: | |||
await to_thread.run_sync( | |||
process_create_tenant, | |||
request, | |||
await request.body(), | |||
body, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks the async model. FastAPI blocks the main thread while serializing bodies, which impacts performance in a big way. I think we can expose types in the __init__
. Let me check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![image](https://private-user-images.githubusercontent.com/1157440/342436676-1bfdee3e-5f68-42f2-a82a-20bf6272c782.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTk2MDA3NzEsIm5iZiI6MTcxOTYwMDQ3MSwicGF0aCI6Ii8xMTU3NDQwLzM0MjQzNjY3Ni0xYmZkZWUzZS01ZjY4LTQyZjItYTgyYS0yMGJmNjI3MmM3ODIucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDYyOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA2MjhUMTg0NzUxWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9OTFmM2U3YTJlYThmNTUyYzY3MTAwMDkzN2ViZDlkMDZiY2Q1YjE5MjU0OTYyOTViNWJjZjIwMzZiY2QwZmQ4NiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.ZnCmcrT1ufupcyBy0Hpog1r8nwQcJ_sfBM5V2fECXvI)
with the following:
self.router.add_api_route(
"/api/v1/collections/{collection_id}/add",
self.add,
methods=["POST"],
status_code=status.HTTP_201_CREATED,
response_model=bool,
)
...
@trace_method("FastAPI.add", OpenTelemetryGranularity.OPERATION)
async def add(self, request: Request, collection_id: str, add_request: AddEmbedding = Body(...)) -> bool:
try:
def process_add(request: Request, raw_body: bytes) -> bool:
add = AddEmbedding.model_validate(orjson.loads(raw_body))
self.auth_and_get_tenant_and_database_for_request(
request.headers,
AuthzAction.ADD,
None,
None,
collection_id,
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tazarov oh interesting. this is good to know! please let m know
Update fastapi types to publish through openapi spec for successful typescript client codegen
notes
ALLOW_RESET=True
on CLIInclude
instead of renaming itIncludeEnum
- minimize the number of changes