-
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
[ENH]: Improve tenant and DB validation handling #1494
base: main
Are you sure you want to change the base?
[ENH]: Improve tenant and DB validation handling #1494
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
chromadb/api/client.py
Outdated
) | ||
if ex.response.status_code == 404: | ||
raise ValueError( | ||
f"It appears your Chroma server is running an older version of Chroma {self.get_version()}. " |
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 error message could be a bit confusing, its only printing the client version right?
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.
Oh I see, its saying the validation routes don't exist, maybe we can make that clearer?
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.
Now it looks like this:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/tazarov/experiments/chroma-experiments/oss/1480-better-tenant-db-validation/chromadb/__init__.py", line 182, in HttpClient
return ClientCreator(tenant=tenant, database=database, settings=settings)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/tazarov/experiments/chroma-experiments/oss/1480-better-tenant-db-validation/chromadb/api/client.py", line 148, in __init__
self._validate_tenant_database(tenant=tenant, database=database)
File "/Users/tazarov/experiments/chroma-experiments/oss/1480-better-tenant-db-validation/chromadb/api/client.py", line 444, in _validate_tenant_database
raise ValueError(
ValueError: It appears you are using newer version of Chroma client (v0.4.18) that is not compatible with Chroma server (v0.4.12). Please upgrade your server to the latest version.
try: | ||
resp.raise_for_status() | ||
except requests.HTTPError: | ||
raise (Exception(resp.text)) |
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.
Removed wrapping specific error (HTTP) into a generic Exception wrapper.
- 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: chroma-core#1480
TODO - regen of the API is needed for error message. Refs: chroma-core#1480
- Exported Settings in __all__ Refs: chroma-core#1480
Ref: #1480
Description of changes
Summarize the changes made by this PR.
Test plan
How are these changes tested?
pytest
for pythonDocumentation Changes
N/A