updates from reviews

Author: markurtz

src/guidellm/scheduler/objects.py

@@ -11,12 +11,12 @@
 
 import time
 import uuid
-from abc import ABC, abstractmethod
 from collections.abc import AsyncIterator
 from typing import (
     Any,
     Generic,
     Literal,
+    Protocol,
     TypeVar,
     Union,
 )
@@ -59,7 +59,10 @@
 
 
 class RequestSchedulerTimings(StandardBaseModel):
-    """Scheduler-level timing measurements for request lifecycle tracking."""
+    """
+    Scheduler-level timing measurements for request lifecycle tracking.
+    All timestamps are expected to be in Unix time (seconds since epoch).
+    """
 
     targeted_start: float | None = Field(
         default=None,
@@ -89,7 +92,10 @@ class RequestSchedulerTimings(StandardBaseModel):
 
 
 class MeasuredRequestTimings(StandardBaseModel):
-    """Base timing measurements for backend request processing."""
+    """
+    Base timing measurements for backend request processing.
+    All timestamps are expected to be in Unix time (seconds since epoch).
+    """
 
     request_start: float | None = Field(
         default=None, description="When the backend began processing the request"
@@ -203,7 +209,7 @@ def model_copy(self, **kwargs) -> ScheduledRequestInfo:  # type: ignore[override
         )
 
 
-class BackendInterface(ABC, Generic[RequestT, MeasuredRequestTimingsT, ResponseT]):
+class BackendInterface(Protocol, Generic[RequestT, MeasuredRequestTimingsT, ResponseT]):
     """
     Abstract interface for request processing backends.
 
@@ -227,52 +233,44 @@ async def resolve(self, request, request_info, history=None):
     """
 
     @property
-    @abstractmethod
     def processes_limit(self) -> int | None:
         """
         :return: Maximum worker processes supported, or None if unlimited
         """
 
     @property
-    @abstractmethod
     def requests_limit(self) -> int | None:
         """
         :return: Maximum concurrent requests supported, or None if unlimited
         """
 
     @property
-    @abstractmethod
     def info(self) -> dict[str, Any]:
         """
         :return: Backend metadata including model initialization and configuration
         """
-        ...
 
-    @abstractmethod
     async def process_startup(self) -> None:
         """
         Perform backend initialization and startup procedures.
 
         :raises: Implementation-specific exceptions for startup failures.
         """
 
-    @abstractmethod
     async def validate(self) -> None:
         """
         Validate backend configuration and operational status.
 
         :raises: Implementation-specific exceptions for validation failures.
         """
 
-    @abstractmethod
     async def process_shutdown(self) -> None:
         """
         Perform backend cleanup and shutdown procedures.
 
         :raises: Implementation-specific exceptions for shutdown failures.
         """
 
-    @abstractmethod
     async def resolve(
         self,
         request: RequestT,

tests/unit/scheduler/test_objects.py

@@ -2,7 +2,6 @@
 
 import inspect
 import typing
-from abc import ABC
 from collections.abc import AsyncIterator
 from typing import Any, Optional, TypeVar, Union
 
@@ -76,13 +75,6 @@ def test_multi_turn_request_t():
 class TestBackendInterface:
     """Test the BackendInterface abstract base class."""
 
-    @pytest.mark.smoke
-    def test_is_abstract_base_class(self):
-        """Test that BackendInterface is an ABC and cannot be instantiated directly."""
-        assert issubclass(BackendInterface, ABC)
-        with pytest.raises(TypeError, match="Can't instantiate abstract class"):
-            BackendInterface()
-
     @pytest.mark.smoke
     def test_abstract_methods_defined(self):
         """Test that all expected abstract methods are defined."""
@@ -112,17 +104,17 @@ def test_abstract_methods_defined(self):
     def test_generic_type_parameters(self):
         """Test that BackendInterface has the correct generic type parameters."""
         orig_bases = BackendInterface.__orig_bases__
-        abc_base = None
+        protocol_base = None
         generic_base = None
 
         for base in orig_bases:
             if hasattr(base, "__origin__"):
                 if base.__origin__ is typing.Generic:
                     generic_base = base
-            elif base.__name__ == "ABC":
-                abc_base = base
+            elif base.__name__ == "Protocol":
+                protocol_base = base
 
-        assert abc_base is not None, "Should inherit from ABC"
+        assert protocol_base is not None, "Should inherit from Protocol"
         assert generic_base is not None, "Should inherit from Generic"
 
         if hasattr(generic_base, "__args__"):
@@ -132,30 +124,6 @@ def test_generic_type_parameters(self):
             expected_names = ["RequestT", "MeasuredRequestTimingsT", "ResponseT"]
             assert param_names == expected_names
 
-    @pytest.mark.sanity
-    def test_invalid_implementation(self):
-        """Test that a concrete implementation must implement all abstract methods."""
-
-        class PartialBackend(BackendInterface):
-            @property
-            def processes_limit(self):
-                return 1
-
-            @property
-            def requests_limit(self):
-                return 10
-
-            def info(self):
-                return {}
-
-            async def process_startup(self):
-                pass
-
-            # Missing: validate, process_shutdown, resolve
-
-        with pytest.raises(TypeError, match="Can't instantiate abstract class"):
-            PartialBackend()
-
     @pytest.mark.smoke
     def test_implementation_construction(self):
         """Test that a complete concrete implementation can be instantiated."""