simplified error handling to surface more errors, added test that checks errors

Author: Diego Ardila

conftest.py

@@ -29,34 +29,8 @@
 
 
 @pytest.fixture(scope="session")
-def monkeypatch_session(request):
-    """This workaround is needed to allow monkeypatching in session-scoped fixtures.
-
-    See https://github.com/pytest-dev/pytest/issues/363
-    """
-    from _pytest.monkeypatch import MonkeyPatch
-
-    mpatch = MonkeyPatch()
-    yield mpatch
-    mpatch.undo()
-
-
-@pytest.fixture(scope="session")
-def CLIENT(monkeypatch_session):
+def CLIENT():
     client = nucleus.NucleusClient(API_KEY)
-
-    # Change _make_request to raise AsssertionErrors when the
-    # HTTP status code is not successful, so that tests fail if
-    # the request was unsuccessful.
-    def _make_request_patch(
-        payload: dict, route: str, requests_command=requests.post
-    ) -> dict:
-        response = client._make_request_raw(payload, route, requests_command)
-        if response.status_code not in SUCCESS_STATUS_CODES:
-            response.raise_for_status()
-        return response.json()
-
-    monkeypatch_session.setattr(client, "make_request", _make_request_patch)
     return client
 
 

nucleus/__init__.py

@@ -132,6 +132,7 @@
     ModelRunCreationError,
     DatasetItemRetrievalError,
     NotFoundError,
+    NucleusAPIError,
 )
 
 logger = logging.getLogger(__name__)
@@ -146,15 +147,21 @@ class NucleusClient:
     Nucleus client.
     """
 
-    def __init__(self, api_key: str, use_notebook: bool = False):
+    def __init__(
+        self,
+        api_key: str,
+        use_notebook: bool = False,
+        endpoint=NUCLEUS_ENDPOINT,
+    ):
         self.api_key = api_key
         self.tqdm_bar = tqdm.tqdm
+        self.endpoint = endpoint
         self._use_notebook = use_notebook
         if use_notebook:
             self.tqdm_bar = tqdm_notebook.tqdm
 
     def __repr__(self):
-        return f"NucleusClient(api_key='{self.api_key}', use_notebook={self._use_notebook})"
+        return f"NucleusClient(api_key='{self.api_key}', use_notebook={self._use_notebook}, endpoint='{self.endpoint}'')"
 
     def __eq__(self, other):
         if self.api_key == other.api_key:
@@ -1080,7 +1087,7 @@ def _make_grequest(
         sess.mount("https://", adapter)
         sess.mount("http://", adapter)
 
-        endpoint = f"{NUCLEUS_ENDPOINT}/{route}"
+        endpoint = f"{self.endpoint}/{route}"
         logger.info("Posting to %s", endpoint)
 
         if local:
@@ -1103,18 +1110,17 @@ def _make_grequest(
         return post
 
     def _make_request_raw(
-        self, payload: dict, route: str, requests_command=requests.post
+        self, payload: dict, endpoint: str, requests_command=requests.post
     ):
         """
         Makes a request to Nucleus endpoint. This method returns the raw
         requests.Response object which is useful for unit testing.
 
         :param payload: given payload
-        :param route: route for the request
+        :param endpoint: endpoint + route for the request
         :param requests_command: requests.post, requests.get, requests.delete
         :return: response
         """
-        endpoint = f"{NUCLEUS_ENDPOINT}/{route}"
         logger.info("Posting to %s", endpoint)
 
         response = requests_command(
@@ -1140,12 +1146,14 @@ def make_request(
         :param requests_command: requests.post, requests.get, requests.delete
         :return: response JSON
         """
-        response = self._make_request_raw(payload, route, requests_command)
+        endpoint = f"{self.endpoint}/{route}"
 
-        if getattr(response, "status_code") not in SUCCESS_STATUS_CODES:
-            logger.warning(response)
+        response = self._make_request_raw(payload, endpoint, requests_command)
 
-        if response.status_code == 404:
-            raise response.raise_for_status()
+        if not response.ok:
+            self.handle_bad_response(endpoint, requests_command, response)
 
         return response.json()
+
+    def handle_bad_response(self, endpoint, requests_command, response):
+        raise NucleusAPIError(endpoint, requests_command, response)

nucleus/errors.py

@@ -22,3 +22,12 @@ class DatasetItemRetrievalError(Exception):
     def __init__(self, message="Could not retrieve dataset items"):
         self.message = message
         super().__init__(self.message)
+
+
+class NucleusAPIError(Exception):
+    def __init__(self, endpoint, command, response):
+        message = f"Tried to {command.__name__} {endpoint}, but received {response.status_code}: {response.reason}."
+        if hasattr(response, "text"):
+            if response.text:
+                message += f"\nThe detailed error is:\n{response.text}"
+        super().__init__(message)

tests/test_dataset.py

@@ -1,4 +1,5 @@
 import pytest
+import os
 
 from helpers import (
     TEST_SLICE_NAME,
@@ -8,7 +9,13 @@
     reference_id_from_url,
 )
 
-from nucleus import Dataset, DatasetItem, UploadResponse, NucleusClient
+from nucleus import (
+    Dataset,
+    DatasetItem,
+    UploadResponse,
+    NucleusClient,
+    NucleusAPIError,
+)
 from nucleus.constants import (
     NEW_ITEMS,
     UPDATED_ITEMS,
@@ -18,6 +25,8 @@
     DATASET_ID_KEY,
 )
 
+TEST_AUTOTAG_DATASET = "ds_bz43jm2jwm70060b3890"
+
 
 def test_reprs():
     # Have to define here in order to have access to all relevant objects
@@ -130,10 +139,20 @@ def test_dataset_list_autotags(CLIENT, dataset):
 
 
 def test_dataset_export_autotag_scores(CLIENT):
-    # Pandoc dataset.
-    client.get_dataset("ds_bwhjbyfb8mjj0ykagxf0")
-
+    # This test can only run for the test user who has an indexed dataset.
     # TODO: if/when we can create autotags via api, create one instead.
-    dataset.autotag_scores(autotag_name="red_car_v2")
+    if os.environ.get("HAS_ACCESS_TO_TEST_DATA", False):
+        dataset = CLIENT.get_dataset(TEST_AUTOTAG_DATASET)
+
+        with pytest.raises(NucleusAPIError) as api_error:
+            dataset.autotag_scores(autotag_name="NONSENSE_GARBAGE")
+        assert (
+            f"The autotag NONSENSE_GARBAGE was not found in dataset {TEST_AUTOTAG_DATASET}"
+            in str(api_error.value)
+        )
+
+        scores = dataset.autotag_scores(autotag_name="TestTag")
 
-    # TODO: add some asserts?
+        for column in ["dataset_item_ids", "ref_ids", "scores"]:
+            assert column in scores
+            assert len(scores[column]) > 0