feat(router): expose virtual model running details - #5397
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors how token router configurations are handled and described in the supervisor, introducing a schema-complete normalization for UI-facing responses and adding support for describing both virtual and physical running models. It also includes comprehensive test coverage for these changes. The review feedback suggests several defensive programming improvements in xinference/core/supervisor.py to prevent potential runtime exceptions, such as using the robust _safe_public_int helper to parse the configuration revision safely and using .get() instead of direct bracket access to avoid potential KeyErrors when retrieving router and virtual model UIDs.
| if not ( | ||
| instance.get("online") | ||
| and instance.get("status") == "ready" | ||
| and not instance.get("config_error") | ||
| and acked_revision >= int(config.get("revision", 0)) | ||
| and isinstance(endpoint, str) | ||
| and endpoint.startswith(("http://", "https://")) | ||
| ): |
There was a problem hiding this comment.
If config.get("revision") is None or a non-integer string, calling int() directly on it will raise a TypeError or ValueError, crashing the health check. Use the robust self._safe_public_int helper to safely parse the revision value with a fallback default.
| if not ( | |
| instance.get("online") | |
| and instance.get("status") == "ready" | |
| and not instance.get("config_error") | |
| and acked_revision >= int(config.get("revision", 0)) | |
| and isinstance(endpoint, str) | |
| and endpoint.startswith(("http://", "https://")) | |
| ): | |
| if not ( | |
| instance.get("online") | |
| and instance.get("status") == "ready" | |
| and not instance.get("config_error") | |
| and acked_revision >= self._safe_public_int(config.get("revision"), 0) | |
| and isinstance(endpoint, str) | |
| and endpoint.startswith(("http://", "https://")) | |
| ): |
There was a problem hiding this comment.
Fixed in bedc100. The config revision is parsed with _safe_public_int, and invalid revisions now fail closed so no runtime is considered ready. Added regression coverage for None and non-integer revisions.
| self, config: Dict[str, Any] | ||
| ) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]], List[Dict[str, Any]]]: | ||
| """Return all, effectively ready, and controllably ready Runtimes.""" | ||
| instances = self._token_router_registry.list(config["router_uid"]) |
There was a problem hiding this comment.
To prevent potential KeyError exceptions if config is missing the router_uid key, use .get() to retrieve it defensively.
| instances = self._token_router_registry.list(config["router_uid"]) | |
| instances = self._token_router_registry.list(config.get("router_uid")) |
There was a problem hiding this comment.
Fixed in bedc100. router_uid is validated before querying the registry. We intentionally return an empty instance list for a missing/invalid UID because RouterRuntimeRegistry.list(None) returns all Router instances and would cause cross-Router state leakage. Added a regression test for this case.
| return { | ||
| "model_name": config["virtual_model_uid"], | ||
| "model_type": config.get("model_type", "LLM"), | ||
| "model_engine": "token_router", | ||
| "model_ability": ["chat"], | ||
| "model_kind": "virtual", | ||
| "virtual_model_type": "token_router", | ||
| "router_uid": config["router_uid"], |
There was a problem hiding this comment.
To adhere to defensive programming practices and prevent potential KeyError exceptions if the keys are missing from config, use .get() instead of direct bracket access for virtual_model_uid and router_uid.
| return { | |
| "model_name": config["virtual_model_uid"], | |
| "model_type": config.get("model_type", "LLM"), | |
| "model_engine": "token_router", | |
| "model_ability": ["chat"], | |
| "model_kind": "virtual", | |
| "virtual_model_type": "token_router", | |
| "router_uid": config["router_uid"], | |
| return { | |
| "model_name": config.get("virtual_model_uid"), | |
| "model_type": config.get("model_type", "LLM"), | |
| "model_engine": "token_router", | |
| "model_ability": ["chat"], | |
| "model_kind": "virtual", | |
| "virtual_model_type": "token_router", | |
| "router_uid": config.get("router_uid"), |
There was a problem hiding this comment.
Fixed in bedc100. Missing or non-string virtual_model_uid and router_uid values are now rendered as empty strings instead of raising KeyError, preserving stable public response types. Added regression coverage.
2 participants
Summary
GET /v1/models/{model_uid}through the unified running-model descriptorDependency
Validation
pytest -q xinference/api/tests/test_token_router_dispatch.py xinference/api/tests/test_token_router_api.pypre-commit run --files xinference/core/supervisor.py xinference/api/restful_api.py xinference/api/tests/test_token_router_dispatch.py xinference/api/tests/test_token_router_api.py