Skip to content
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

[Bug]: Inconsistent upsert/update API responses #1466

Closed
tazarov opened this issue Dec 5, 2023 · 8 comments
Closed

[Bug]: Inconsistent upsert/update API responses #1466

tazarov opened this issue Dec 5, 2023 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@tazarov
Copy link
Contributor

tazarov commented Dec 5, 2023

What happened?

In client/server mode, the API does not return a proper response for update and upsert collection ops when there is a mismatch in embedding and collection dimensions.

Versions

Chroma 0.4.18, Python 3.11

Relevant log output

Response for add:

Exception: {"detail":"Embedding dimension 384 does not match collection dimensionality 1536"}

Response for update and upsert:

HTTPError: 400 Client Error: Bad Request for url: http://localhost:8000/api/v1/collections/1cc231ef-b34b-463a-9fc7-69b3184ba2a6/update
@tazarov tazarov added the bug Something isn't working label Dec 5, 2023
@tazarov
Copy link
Contributor Author

tazarov commented Dec 5, 2023

@shardulkulkarni95
Copy link
Contributor

Can you assign this to me?

@HammadB
Copy link
Collaborator

HammadB commented Dec 6, 2023

Of course! Thanks.

@shardulkulkarni95
Copy link
Contributor

shardulkulkarni95 commented Dec 6, 2023

raised PR. #1470

Please review.

@ChrisLahaye
Copy link

Also noticed that the upsert endpoint returns null since chroma >=0.4.16 instead of true/false. Will #1470 also fix this?

@tazarov
Copy link
Contributor Author

tazarov commented Dec 7, 2023

@ChrisLahaye, I don't think it will. The change you are referring to was introduced way way back in May-23 - https://github.com/chroma-core/chroma/pull/524/files#diff-1782a536c97875ef5659801ed08b6157216affe33150a87e6adbae02844fc56bL209

Returning None is still the desired behaviour until we switch the response scheme.

@ChrisLahaye
Copy link

@tazarov Thank you for that information! Great to know that it is the intended behavior and not a bug. The chromadb JavaScript package still uses a promise boolean as return type for the upsert function though.

HammadB pushed a commit that referenced this issue Dec 7, 2023
## Description of changes

*Summarize the changes made by this PR.*
 - Improvements & Bug fixes
- Error response was containing status code and stack massage altered to
chroma error to make uniform response to user. followed same logic as
add.

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

To reproduce:

https://gist.github.com/tazarov/35539b3a52bd02814edc0b565ec227eb

## 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)?*
@tazarov
Copy link
Contributor Author

tazarov commented Dec 11, 2023

Fix for this implemented. Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants