Conversation
| from typing import Any, TYPE_CHECKING | ||
|
|
||
| try: | ||
| if TYPE_CHECKING: | ||
| from inferedge_moss import ( | ||
| AddDocumentsOptions, | ||
| DocumentInfo, | ||
| GetDocumentsOptions, | ||
| IndexInfo, | ||
| MossClient as _InferEdgeMossClient, | ||
| QueryOptions, | ||
| SearchResult, | ||
| ) | ||
| except ImportError: | ||
| _InferEdgeMossClient = None | ||
| else: | ||
| try: | ||
| from inferedge_moss import ( | ||
| AddDocumentsOptions, | ||
| DocumentInfo, | ||
| GetDocumentsOptions, | ||
| IndexInfo, | ||
| MossClient as _InferEdgeMossClient, | ||
| QueryOptions, |
There was a problem hiding this comment.
used TYPECHECK to safely handle during type checking.
There was a problem hiding this comment.
when i remove it and keep it clean, it fails on make type-check
| DocumentInfo, | ||
| GetDocumentsOptions, | ||
| IndexInfo, | ||
| MossClient as _InferEdgeMossClient, |
There was a problem hiding this comment.
Also do not use the string Inferedge.
There was a problem hiding this comment.
Should i just use MossClient as just client?
livekit-plugins/livekit-plugins-moss/livekit/plugins/moss/MossClient.py:68: error: Name "MossClient" already defined (possibly by an import) [no-redef]
Found 1 error in 1 file (checked 478 source files)
✗ Type checking failed
make: *** [type-check] Error 1
Get the type-check error if i don't reference it and directly use mossclient
There was a problem hiding this comment.
Pull request overview
This PR updates the livekit-plugins-moss integration to align with the latest inferedge-moss SDK (version 1.0.0b12). The changes enhance the plugin's API surface by exposing new query options and index management capabilities while maintaining full backward compatibility with existing code.
Changes:
- Bumped inferedge-moss dependency from 1.0.0b2 to 1.0.0b12
- Added QueryOptions export for advanced query configuration
- Enhanced index loading with auto-refresh capabilities and added unload_index method
- Made create_index model_id parameter optional and updated query method to support QueryOptions
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pyproject.toml | Updated inferedge-moss dependency version from 1.0.0b2 to 1.0.0b12 |
| __init__.py | Added QueryOptions to module exports |
| MossClient.py | Refactored imports with TYPE_CHECKING pattern, added QueryOptions support, enhanced load_index with auto-refresh parameters, added unload_index method, improved variable naming and documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result = await self._client.load_index( # type: ignore[call-arg] | ||
| index_name, auto_refresh, polling_interval_in_seconds | ||
| ) |
There was a problem hiding this comment.
The type ignore comment suggests that the type stubs for inferedge-moss version 1.0.0b12 may not yet reflect these new parameters (auto_refresh and polling_interval_in_seconds). Consider verifying that these parameters match the actual API signature of the underlying library, and if the type stubs are available but incomplete, consider contributing updated stubs to the inferedge-moss project.
There was a problem hiding this comment.
@HarshaNalluru is there anything i need to change here?
| """Unload an index from memory.""" | ||
|
|
||
| logger.debug("unloading moss index", extra={"index": index_name}) | ||
| await self._client.unload_index(index_name) # type: ignore[attr-defined] |
There was a problem hiding this comment.
The type ignore comment indicates that the unload_index method may not be recognized by type checkers. Similar to the load_index method, consider verifying that the method signature matches the actual API and updating type stubs if needed.
| raise RuntimeError( | ||
| "inferedge-moss is required to use MossClient. Install it via `pip install inferedge-moss`." | ||
| "moss is required to use MossClient. Install it via `pip install inferedge-moss`." |
There was a problem hiding this comment.
The error message refers to "moss" but should be consistent with the actual package name "inferedge-moss" as shown in the error message on line 33 and the installation instruction. Consider changing this to "inferedge-moss is required to use MossClient" for consistency.
| "moss is required to use MossClient. Install it via `pip install inferedge-moss`." | |
| "inferedge-moss is required to use MossClient. Install it via `pip install inferedge-moss`." |
CoderOMaster
force-pushed
the
update-moss-plugin-api
branch
from
541cab1 to
8fe13a9
Compare
Updated with latest doc updates.
Ran type checks and linting