Enable configuring HTTP transport retry behavior and add request_timeout parameter to get_detector (#386)

This PR includes two distinct changes.
- Adds a new parameter `http_transport_retries` to the `Groundlight`
constructor, enabling the retry behavior to be modified when
initializing a `Groundlight` instance.
- The retry behavior specified will apply to all API requests made
through the `Groundlight` instance.
- Currently no way is provided to configure the retry behavior
per-request, or to change it once the `Groundlight` instance has been
initialized (this is technically possible but awkward).
- I added a test to verify that we can pass the parameter in and it gets
passed through to the inner API client.
- Adds a `request_timeout` parameter to the `get_detector` method which
enables changing the request timeout values for the request.
- An identical parameter was previously added for `submit_image_query`.
Eventually I think we'll want to make this available for all methods,
but for now we don't need that.
  - I added a test to verify that this parameter works properly.

These changes are intended to be utilized by the edge endpoint to
improve behavior under poor network connectivity.

Author: CoreyEWood

pyproject.toml

@@ -21,7 +21,7 @@ python = ">=3.9,<4.0"
 python-dateutil = "^2.9.0"
 requests = "^2.28.2"
 typer = "^0.15.4"
-urllib3 = "^1.26.9"
+urllib3 = "^2.5.0"
 
 [tool.poetry.group.dev.dependencies]
 datamodel-code-generator = "^0.22.1"

src/groundlight/client.py

@@ -37,6 +37,7 @@
     PaginatedImageQueryList,
 )
 from urllib3.exceptions import InsecureRequestWarning
+from urllib3.util.retry import Retry
 
 from groundlight.binary_labels import Label, convert_internal_label_to_display
 from groundlight.config import API_TOKEN_MISSING_HELP_MESSAGE, API_TOKEN_VARIABLE_NAME, DISABLE_TLS_VARIABLE_NAME
@@ -133,26 +134,31 @@ def __init__(
         endpoint: Optional[str] = None,
         api_token: Optional[str] = None,
         disable_tls_verification: Optional[bool] = None,
+        http_transport_retries: Optional[Union[int, Retry]] = None,
     ):
         """
         Initialize a new Groundlight client instance.
 
         :param endpoint: Optional custom API endpoint URL. If not specified, uses the default Groundlight endpoint.
         :param api_token: Authentication token for API access.
-                        If not provided, will attempt to read from the "GROUNDLIGHT_API_TOKEN" environment variable.
+            If not provided, will attempt to read from the "GROUNDLIGHT_API_TOKEN" environment variable.
         :param disable_tls_verification: If True, disables SSL/TLS certificate verification for API calls.
-                                       When not specified, checks the "DISABLE_TLS_VERIFY" environment variable
-                                       (1=disable, 0=enable). Certificate verification is enabled by default.
+            When not specified, checks the "DISABLE_TLS_VERIFY" environment variable (1=disable, 0=enable).
+            Certificate verification is enabled by default.
 
-                                       Warning: Only disable verification when connecting to a Groundlight Edge
-                                       Endpoint using self-signed certificates. For security, always keep
-                                       verification enabled when using the Groundlight cloud service.
+            Warning: Only disable verification when connecting to a Groundlight Edge Endpoint using self-signed
+            certificates. For security, always keep verification enabled when using the Groundlight cloud service.
+        :param http_transport_retries: Overrides urllib3 `PoolManager` retry policy for HTTP/HTTPS (forwarded to
+            `Configuration.retries`). Not the same as SDK 5xx retries handled by `RequestsRetryDecorator`.
 
         :return: Groundlight client
         """
         # Specify the endpoint
         self.endpoint = sanitize_endpoint_url(endpoint)
         self.configuration = Configuration(host=self.endpoint)
+        if http_transport_retries is not None:
+            # Once we upgrade openapitools to ^7.7.0, retries can be passed into the constructor of Configuration above.
+            self.configuration.retries = http_transport_retries
 
         if not api_token:
             try:
@@ -264,7 +270,11 @@ def _user_is_privileged(self) -> bool:
         obj = self.user_api.who_am_i()
         return obj["is_superuser"]
 
-    def get_detector(self, id: Union[str, Detector]) -> Detector:  # pylint: disable=redefined-builtin
+    def get_detector(
+        self,
+        id: Union[str, Detector],  # pylint: disable=redefined-builtin
+        request_timeout: Optional[Union[float, Tuple[float, float]]] = None,
+    ) -> Detector:
         """
         Get a Detector by id.
 
@@ -275,6 +285,8 @@ def get_detector(self, id: Union[str, Detector]) -> Detector:  # pylint: disable
             print(detector)
 
         :param id: the detector id
+        :param request_timeout: The request timeout for the image query submission API request. Most users will not need
+            to modify this. If not set, the default value will be used.
 
         :return: Detector
         """
@@ -283,7 +295,8 @@ def get_detector(self, id: Union[str, Detector]) -> Detector:  # pylint: disable
             # Short-circuit
             return id
         try:
-            obj = self.detectors_api.get_detector(id=id, _request_timeout=DEFAULT_REQUEST_TIMEOUT)
+            request_timeout = request_timeout if request_timeout is not None else DEFAULT_REQUEST_TIMEOUT
+            obj = self.detectors_api.get_detector(id=id, _request_timeout=request_timeout)
         except NotFoundException as e:
             raise NotFoundError(f"Detector with id '{id}' not found") from e
         return Detector.parse_obj(obj.to_dict())
@@ -662,7 +675,7 @@ def submit_image_query(  # noqa: PLR0913 # pylint: disable=too-many-arguments, t
         inspection_id: Optional[str] = None,
         metadata: Union[dict, str, None] = None,
         image_query_id: Optional[str] = None,
-        request_timeout: Optional[float] = None,
+        request_timeout: Optional[Union[float, Tuple[float, float]]] = None,
     ) -> ImageQuery:
         """
         Evaluates an image with Groundlight. This is the core method for getting predictions about images.
@@ -744,8 +757,8 @@ def submit_image_query(  # noqa: PLR0913 # pylint: disable=too-many-arguments, t
         :param image_query_id: The ID for the image query. This is to enable specific functionality
                             and is not intended for general external use. If not set, a random ID
                             will be generated.
-        :param request_timeout: The total request timeout for the image query submission API request. Most users will
-            not need to modify this. If not set, the default value will be used.
+        :param request_timeout: The request timeout for the image query submission API request. Most users will not need
+            to modify this. If not set, the default value will be used.
 
         :return: ImageQuery with query details and result (if wait > 0)
         :raises ValueError: If wait > 0 when want_async=True

test/integration/test_groundlight.py

@@ -27,7 +27,8 @@
     PaginatedDetectorList,
     PaginatedImageQueryList,
 )
-from urllib3.exceptions import ReadTimeoutError
+from urllib3.exceptions import ConnectTimeoutError, MaxRetryError, ReadTimeoutError
+from urllib3.util.retry import Retry
 
 DEFAULT_CONFIDENCE_THRESHOLD = 0.9
 IQ_IMPROVEMENT_THRESHOLD = 0.75
@@ -80,6 +81,21 @@ def fixture_image() -> str:
     return "test/assets/dog.jpeg"
 
 
+def test_create_groundlight_with_retries():
+    """Verify that the `retries` parameter can be successfully passed to the `Groundlight` constructor."""
+    # Set retries using int value
+    num_retries = 25
+    gl = Groundlight(http_transport_retries=num_retries)
+    assert gl.configuration.retries == num_retries
+    assert gl.api_client.configuration.retries == num_retries
+
+    # Set retries using Retry object
+    retries = Retry(total=num_retries)
+    gl = Groundlight(http_transport_retries=retries)
+    assert gl.configuration.retries.total == retries.total
+    assert gl.api_client.configuration.retries.total == retries.total
+
+
 def test_create_detector(gl: Groundlight):
     name = f"Test {datetime.utcnow()}"  # Need a unique name
     query = "Is there a dog?"
@@ -222,6 +238,21 @@ def test_get_detector(gl: Groundlight, detector: Detector):
     assert isinstance(_detector, Detector)
 
 
+def test_get_detector_with_low_request_timeout(gl: Groundlight, detector: Detector):
+    """
+    Verifies that get_detector respects the request_timeout parameter and raises a MaxRetryError when timeout is
+    low. Verifies that request_timeout parameter can be a float or a tuple.
+    """
+    with pytest.raises(MaxRetryError):
+        # Setting a very low request_timeout value should result in a timeout.
+        # NOTE: request_timeout=0 seems to have special behavior that does not result in a timeout.
+        gl.get_detector(id=detector.id, request_timeout=1e-8)
+
+    with pytest.raises(MaxRetryError):
+        # Ensure a tuple can be passed.
+        gl.get_detector(id=detector.id, request_timeout=(1e-8, 1e-8))
+
+
 def test_get_detector_by_name(gl: Groundlight, detector: Detector):
     _detector = gl.get_detector_by_name(name=detector.name)
     assert str(_detector)
@@ -352,14 +383,18 @@ def test_submit_image_query_with_human_review_param(gl: Groundlight, detector: D
 
 def test_submit_image_query_with_low_request_timeout(gl: Groundlight, detector: Detector, image: str):
     """
-    Test that submit_image_query respects the request_timeout parameter and raises a ReadTimeoutError when timeout is
-    exceeded.
+    Verifies that submit_image_query respects the request_timeout parameter and raises a ConnectTimeoutError or
+    ReadTimeoutError when timeout is low. Verifies that request_timeout parameter can be a float or a tuple.
     """
-    with pytest.raises(ReadTimeoutError):
+    with pytest.raises((ConnectTimeoutError, ReadTimeoutError)):
         # Setting a very low request_timeout value should result in a timeout.
         # NOTE: request_timeout=0 seems to have special behavior that does not result in a timeout.
         gl.submit_image_query(detector=detector, image=image, human_review="NEVER", request_timeout=1e-8)
 
+    with pytest.raises((ConnectTimeoutError, ReadTimeoutError)):
+        # Ensure a tuple can be passed.
+        gl.submit_image_query(detector=detector, image=image, human_review="NEVER", request_timeout=(5, 1e-8))
+
 
 @pytest.mark.skip_for_edge_endpoint(reason="The edge-endpoint does not support passing detector metadata.")
 def test_create_detector_with_metadata(gl: Groundlight):