Endpoint configuration more robust

pyproject.toml

@@ -1,9 +1,9 @@
 [tool.poetry]
 name = "groundlight"
-version = "0.6.4"
+version = "0.7.0"
 license = "MIT"
 readme = "UserGuide.md"
-homepage = "https://groundlight.ai"
+homepage = "https://www.groundlight.ai"
 description = "Build computer vision systems from natural language with Groundlight"
 authors = ["Groundlight AI <support@groundlight.ai>"]
 packages = [

src/groundlight/binary_labels.py

@@ -15,12 +15,12 @@ def internal_labels_for_detector(context: Union[ImageQuery, Detector, str]) -> L
     """Returns an ordered list of class labels as strings.
     These are the versions of labels that the API demands.
     :param context: Can be an ImageQuery, a Detector, or a string-id for one of them."""
-    # TODO: At some point this will need to be an API call, because these will be defined per-detector
+    # NOTE: At some point this will need to be an API call, because these will be defined per-detector
     return ["PASS", "FAIL"]
 
 
 def convert_internal_label_to_display(context: Union[ImageQuery, Detector, str], label: str) -> str:
-    # TODO: Someday we probably do nothing here.
+    # NOTE: Someday we will do nothing here, when the server provides properly named classes.
     upper = label.upper()
     if upper == "PASS":
         return "YES"
@@ -33,7 +33,7 @@ def convert_internal_label_to_display(context: Union[ImageQuery, Detector, str],
 
 
 def convert_display_label_to_internal(context: Union[ImageQuery, Detector, str], label: str) -> str:
-    # TODO: Validate against actually supported labels for the detector
+    # NOTE: In the future we should validate against actually supported labels for the detector
     upper = label.upper()
     if upper == "PASS" or upper == "YES":
         return "PASS"

src/groundlight/client.py

@@ -13,7 +13,7 @@
 from groundlight.binary_labels import convert_display_label_to_internal, convert_internal_label_to_display
 from groundlight.config import API_TOKEN_VARIABLE_NAME, API_TOKEN_WEB_URL, DEFAULT_ENDPOINT
 from groundlight.images import buffer_from_jpeg_file, jpeg_from_numpy
-from groundlight.internalapi import GroundlightApiClient
+from groundlight.internalapi import GroundlightApiClient, sanitize_endpoint_url
 from groundlight.optional_imports import np
 
 logger = logging.getLogger("groundlight.sdk")
@@ -47,7 +47,7 @@ def __init__(self, endpoint: str = DEFAULT_ENDPOINT, api_token: str = None):
             environment variable "GROUNDLIGHT_API_TOKEN".
         """
         # Specify the endpoint
-        self.endpoint = endpoint
+        self.endpoint = sanitize_endpoint_url(endpoint)
         configuration = Configuration(host=endpoint)
 
         if api_token is None:

src/groundlight/config.py

@@ -3,7 +3,7 @@
 API_TOKEN_WEB_URL = "https://app.groundlight.ai/reef/my-account/api-tokens"
 API_TOKEN_VARIABLE_NAME = "GROUNDLIGHT_API_TOKEN"
 
-DEFAULT_ENDPOINT = os.environ.get("GROUNDLIGHT_ENDPOINT", "https://api.groundlight.ai/device-api")
+DEFAULT_ENDPOINT = os.environ.get("GROUNDLIGHT_ENDPOINT", "https://api.groundlight.ai/")
 
 __all__ = [
     "API_TOKEN_WEB_URL",

src/groundlight/internalapi.py

@@ -1,8 +1,8 @@
 import logging
-import os
 import time
 import uuid
 from typing import Dict
+from urllib.parse import urlsplit, urlunsplit
 
 import model
 import requests
@@ -11,6 +11,32 @@
 logger = logging.getLogger("groundlight.sdk")
 
 
+def sanitize_endpoint_url(endpoint: str) -> str:
+    """Takes a URL for an endpoint, and returns a "sanitized" version of it.
+    Currently the production API path must be exactly "/device-api".
+    This allows people to leave that off entirely, or add a trailing slash.
+    Also some future-proofing by allowing "v1" or "v2" or "v3" paths.
+    """
+    parts = urlsplit(endpoint)
+    if (parts.scheme not in ("http", "https")) or (not parts.netloc):
+        raise ValueError(
+            f"Invalid API endpoint {endpoint}.  Unsupported scheme: {parts.scheme}.  Must be http or https, e.g. https://api.groundlight.ai/"
+        )
+    if parts.query or parts.fragment:
+        raise ValueError(f"Invalid API endpoint {endpoint}.  Cannot have query or fragment.")
+    if not parts.path:
+        parts = parts._replace(path="/")
+    if not parts.path.endswith("/"):
+        parts = parts._replace(path=parts.path + "/")
+    if parts.path == "/":
+        parts = parts._replace(path="/device-api/")
+    if parts.path not in ("/device-api/", "/v1/", "/v2/", "/v3/"):
+        logger.warning(f"Configured endpoint {endpoint} does not look right - path '{parts.path}' seems wrong.")
+    out = urlunsplit(parts)
+    out = out[:-1]  # remove trailing slash
+    return out
+
+
 def _generate_request_id():
     # TODO: use a ksuid instead of a uuid.  Most of our API uses ksuids for lots of reasons.
     # But we don't want to just import ksuid because we want to avoid dependency bloat

test/unit/test_config.py

@@ -0,0 +1,39 @@
+import pytest
+
+from groundlight.internalapi import sanitize_endpoint_url
+
+
+def test_invalid_endpoint_config():
+    BAD_ENDPOINTS = [
+        "Just a string",
+        "foo://bar",
+        "http://bar/?foo=123",
+        "http://bar/asdlkfj#123",
+        "hxxp://bar/asdlkfj",
+        "https://api.groundlight.ai/device-api/?bad",
+        "https://api.groundlight.ai/?bad",
+        "https://api.groundlight.ai/#bad",
+    ]
+    for endpoint in BAD_ENDPOINTS:
+        with pytest.raises(ValueError):
+            out = sanitize_endpoint_url(endpoint)
+
+
+def test_endpoint_cleanup():
+    expected = "https://api.groundlight.ai/device-api"
+    assert sanitize_endpoint_url("https://api.groundlight.ai") == expected
+    assert sanitize_endpoint_url("https://api.groundlight.ai/") == expected
+    assert sanitize_endpoint_url("https://api.groundlight.ai/device-api") == expected
+    assert sanitize_endpoint_url("https://api.groundlight.ai/device-api/") == expected
+
+    expected = "https://api.integ.groundlight.ai/device-api"
+    assert sanitize_endpoint_url("https://api.integ.groundlight.ai") == expected
+    assert sanitize_endpoint_url("https://api.integ.groundlight.ai/") == expected
+    assert sanitize_endpoint_url("https://api.integ.groundlight.ai/device-api") == expected
+    assert sanitize_endpoint_url("https://api.integ.groundlight.ai/device-api/") == expected
+
+    expected = "http://localhost:8000/device-api"
+    assert sanitize_endpoint_url("http://localhost:8000") == expected
+    assert sanitize_endpoint_url("http://localhost:8000/") == expected
+    assert sanitize_endpoint_url("http://localhost:8000/device-api") == expected
+    assert sanitize_endpoint_url("http://localhost:8000/device-api/") == expected