Skip to content

Feat/client server version check#6827

Open
Pawansingh3889 wants to merge 8 commits intochroma-core:mainfrom
Pawansingh3889:feat/client-server-version-check
Open

Feat/client server version check#6827
Pawansingh3889 wants to merge 8 commits intochroma-core:mainfrom
Pawansingh3889:feat/client-server-version-check

Conversation

@Pawansingh3889
Copy link
Copy Markdown

Closes #2524

When client and server versions are mismatched, users get confusing errors. This adds a version check on connection so they get a clear warning instead.

New functionality
Server pre_flight_checks now returns server_version alongside max_batch_size
Sync and async HTTP clients compare major.minor versions after connecting
Mismatches trigger a warnings.warn() — no exceptions, nothing breaks

Test plan
Tests pass locally with pytest
Tested with mismatched versions — warning appears as expected

Migration plan
Non-breaking. Old clients ignore the new server_version field. Warning-only, no exceptions.

Observability plan
Uses Python's warnings module — shows up in logs when logging.captureWarnings(True) is set.

Documentation Changes
None needed — the warning message explains the issue and fix.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 5, 2026

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

  • Can you think of any use case in which the code does not behave as intended? Have they been tested?
  • Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
  • If appropriate, are there adequate property based tests?
  • If appropriate, are there adequate unit tests?
  • Should any logging, debugging, tracing information be added or removed?
  • Are error messages user-friendly?
  • Have all documentation changes needed been made?
  • Have all non-obvious changes been commented?

System Compatibility

  • Are there any potential impacts on other parts of the system or backward compatibility?
  • Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

  • Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

@propel-code-bot
Copy link
Copy Markdown
Contributor

propel-code-bot bot commented Apr 5, 2026

Add client/server version mismatch warnings in sync and async Python clients

This PR adds a compatibility check that compares major.minor versions between the Python client and server and emits a warnings.warn(...) message when they differ. The check is implemented in both FastAPI and AsyncFastAPI clients, with failure handling designed to be non-breaking (logger.debug(...) on exceptions rather than raising).

In the sync client, the check is now run during initialization after auth headers are applied, and it uses a dedicated 5-second timeout for the /version call to avoid indefinite blocking from the shared timeout=None session. The PR also includes cleanup from review iteration, including restoring decorators on get_version methods and removing accidental submodule artifacts (with sql-test-repo now ignored in .gitignore).

This summary was automatically generated by @propel-code-bot

Copy link
Copy Markdown
Contributor

@propel-code-bot propel-code-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good feature direction, but key initialization and error-handling gaps currently make version checks unreliable, especially for authenticated and async clients.

Status: Changes Suggested | Risk: Medium

Issues Identified & Suggestions
  • Version check runs before auth headers; move after auth setup: chromadb/api/fastapi.py
  • Async version check not invoked; call from async startup path: chromadb/api/async_fastapi.py
  • Broad exception swallowing hides failures; narrow/log exceptions: chromadb/api/fastapi.py, chromadb/api/async_fastapi.py
  • Remove redundant deferred version imports; use module-level value: chromadb/api/fastapi.py, chromadb/api/async_fastapi.py
Review Details

📁 3 files reviewed | 💬 6 comments

👍 / 👎 individual comments to help improve reviews for you

@trace_method("AsyncFastAPI.get_version", OpenTelemetryGranularity.OPERATION)
@override

async def _check_version_compatibility(self) -> None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

[Logic] _check_version_compatibility is an async method but AsyncFastAPI.__init__ is synchronous — Python __init__ cannot await coroutines. As written, this method is never called during construction of the async client (there is no self._check_version_compatibility() call in AsyncFastAPI.__init__). The version check is completely inoperative for the async client.

To invoke this on startup, it would need to be called from an async entry point such as __aenter__ or a factory create() classmethod. For example:

async def __aenter__(self) -> "AsyncFastAPI":
    self._get_client()
    await self._check_version_compatibility()
    return self

Alternatively, expose a synchronous wrapper or document that callers must invoke it manually after await-ing construction.

Context for Agents
`_check_version_compatibility` is an `async` method but `AsyncFastAPI.__init__` is synchronous — Python `__init__` cannot `await` coroutines. As written, this method is never called during construction of the async client (there is no `self._check_version_compatibility()` call in `AsyncFastAPI.__init__`). The version check is completely inoperative for the async client.

To invoke this on startup, it would need to be called from an async entry point such as `__aenter__` or a factory `create()` classmethod. For example:
```python
async def __aenter__(self) -> "AsyncFastAPI":
    self._get_client()
    await self._check_version_compatibility()
    return self
```
Alternatively, expose a synchronous wrapper or document that callers must invoke it manually after `await`-ing construction.

File: chromadb/api/async_fastapi.py
Line: 798

@Pawansingh3889
Copy link
Copy Markdown
Author

Fixed in the latest push:

Version check now runs after auth headers are set up
Using module-level version instead of deferred imports
except: pass replaced with logger.debug() for visibility
For the async client — happy to add an aenter hook if that's the preferred approach. Let me know.

propel-code-bot[bot]

This comment was marked as outdated.

@Pawansingh3889
Copy link
Copy Markdown
Author

Fixed — version check now uses a dedicated 5s timeout instead of the session default. If the server is slow or unreachable, it logs at debug level and doesn't block startup.

propel-code-bot[bot]

This comment was marked as outdated.

@Pawansingh3889
Copy link
Copy Markdown
Author

Good catch — removed server_version from pre-flight checks since the client already hits /version directly. No point adding fields nothing reads.

propel-code-bot[bot]

This comment was marked as outdated.

propel-code-bot[bot]

This comment was marked as outdated.

@Pawansingh3889
Copy link
Copy Markdown
Author

Fixed — decorators restored to get_version methods in both sync and async clients.

propel-code-bot[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@propel-code-bot propel-code-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues were identified; the version-compatibility check appears well-integrated and non-breaking.

Status: No Issues Found | Risk: Low

Review Details

📁 3 files reviewed | 💬 0 comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ENH] Client / Server versioning compatibility

1 participant