Merge pull request #290 from nbateshaus/handle_http_204

Handle empty HTTP 204 responses for already-completed operations.

CHANGELOG.md

@@ -1,4 +1,6 @@
 ## 0.10.0-dev
+  **BUG FIXES**
+  - [#293](https://github.com/Datatamer/tamr-client/issues/293) Better handling for HTTP 204 on already up-to-date operations
 
 ## 0.9.0
   **BREAKING CHANGES**

tamr_unify_client/dataset/profile.py

@@ -77,8 +77,8 @@ def refresh(self, **options):
         :returns: The refresh operation.
         :rtype: :class:`~tamr_unify_client.operation.Operation`
         """
-        op_json = self.client.post(self.api_path + ":refresh").successful().json()
-        op = Operation.from_json(self.client, op_json)
+        response = self.client.post(self.api_path + ":refresh").successful()
+        op = Operation.from_response(self.client, response)
         return op.apply_options(**options)
 
     def __repr__(self) -> str:

tamr_unify_client/dataset/resource.py

@@ -144,8 +144,8 @@ def refresh(self, **options):
         :returns: The refresh operation.
         :rtype: :class:`~tamr_unify_client.operation.Operation`
         """
-        op_json = self.client.post(self.api_path + ":refresh").successful().json()
-        op = Operation.from_json(self.client, op_json)
+        response = self.client.post(self.api_path + ":refresh").successful()
+        op = Operation.from_response(self.client, response)
         return op.apply_options(**options)
 
     def profile(self):
@@ -174,10 +174,8 @@ def create_profile(self, **options):
         :return: The operation to create the profile.
         :rtype: :class:`~tamr_unify_client.operation.Operation`
         """
-        op_json = (
-            self.client.post(self.api_path + "/profile:refresh").successful().json()
-        )
-        op = Operation.from_json(self.client, op_json)
+        response = self.client.post(self.api_path + "/profile:refresh").successful()
+        op = Operation.from_response(self.client, response)
         return op.apply_options(**options)
 
     def records(self):

tamr_unify_client/mastering/estimated_pair_counts.py

@@ -63,8 +63,8 @@ def refresh(self, **options):
         :returns: The refresh operation.
         :rtype: :class:`~tamr_unify_client.operation.Operation`
         """
-        op_json = self.client.post(self.api_path + ":refresh").successful().json()
-        op = Operation.from_json(self.client, op_json)
+        response = self.client.post(self.api_path + ":refresh").successful()
+        op = Operation.from_response(self.client, response)
         return op.apply_options(**options)
 
     def __repr__(self) -> str:

tamr_unify_client/operation.py

@@ -18,6 +18,48 @@ class Operation(BaseResource):
     def from_json(cls, client, resource_json, api_path=None):
         return super().from_data(client, resource_json, api_path)
 
+    @classmethod
+    def from_response(cls, client, response):
+        """
+        Handle idiosyncrasies in constructing Operations from Tamr responses.
+        When a Tamr API call would start an operation, but all results that would be
+        produced by that operation are already up-to-date, Tamr returns `HTTP 204 No Content`
+
+        To make it easy for client code to handle these API responses without checking
+        the response code, this method will either construct an Operation, or a
+        dummy `NoOp` operation representing the 204 Success response.
+
+        :param client: Delegate underlying API calls to this client.
+        :type client: :class:`~tamr_unify_client.Client`
+        :param response: HTTP Response from the request that started the operation.
+        :type response: :class:`requests.Response`
+        :return: Operation
+        :rtype: :class:`~tamr_unify_client.operation.Operation`
+        """
+        if response.status_code == 204:
+            # Operation was successful, but the response contains no content.
+            # Create a dummy operation to represent this.
+            _never = "0000-00-00T00:00:00.000Z"
+            _description = """Tamr returned HTTP 204 for this operation, indicating that all
+                results that would be produced by the operation are already up-to-date."""
+            resource_json = {
+                "id": "-1",
+                "type": "NOOP",
+                "description": _description,
+                "status": {
+                    "state": "SUCCEEDED",
+                    "startTime": _never,
+                    "endTime": _never,
+                    "message": "",
+                },
+                "created": {"username": "", "time": _never, "version": "-1"},
+                "lastModified": {"username": "", "time": _never, "version": "-1"},
+                "relativeId": "operations/-1",
+            }
+        else:
+            resource_json = response.json()
+        return Operation.from_json(client, resource_json)
+
     def apply_options(self, asynchronous=False, **options):
         """Applies operation options to this operation.
 

tests/unit/test_operation.py

@@ -0,0 +1,95 @@
+from urllib.parse import urljoin
+
+import pytest
+from requests import HTTPError
+import responses
+
+
+from tamr_unify_client.operation import Operation
+
+
+@pytest.fixture
+def client():
+    from tamr_unify_client import Client
+    from tamr_unify_client.auth import UsernamePasswordAuth
+
+    auth = UsernamePasswordAuth("username", "password")
+    tamr = Client(auth)
+    return tamr
+
+
+def full_url(client, endpoint):
+    return urljoin(client.origin + client.base_path, endpoint)
+
+
+op_1_json = {
+    "id": "1",
+    "type": "SPARK",
+    "description": "Profiling [dataset] attributes.",
+    "status": {
+        "state": "SUCCEEDED",
+        "startTime": "2019-08-28T18:51:06.856Z",
+        "endTime": "2019-08-28T18:53:08.204Z",
+        "message": "",
+    },
+    "created": {
+        "username": "admin",
+        "time": "2019-08-28T18:50:35.582Z",
+        "version": "17",
+    },
+    "lastModified": {
+        "username": "system",
+        "time": "2019-08-28T18:53:08.950Z",
+        "version": "40",
+    },
+    "relativeId": "operations/1",
+}
+
+
+def test_operation_from_json(client):
+    alias = "operations/123"
+    op1 = Operation.from_json(client, op_1_json, alias)
+    assert op1.api_path == alias
+    assert op1.relative_id == op_1_json["relativeId"]
+    assert op1.resource_id == "1"
+    assert op1.type == op_1_json["type"]
+    assert op1.description == op_1_json["description"]
+    assert op1.status == op_1_json["status"]
+    assert op1.state == "SUCCEEDED"
+    assert op1.succeeded
+
+
+@responses.activate
+def test_operation_from_response(client):
+    responses.add(responses.GET, full_url(client, "operations/1"), json=op_1_json)
+
+    op1 = Operation.from_response(client, client.get("operations/1").successful())
+
+    assert op1.resource_id == "1"
+    assert op1.succeeded
+
+
+@responses.activate
+def test_operation_from_response_noop(client):
+    responses.add(responses.GET, full_url(client, "operations/2"), status=204)
+    responses.add(responses.GET, full_url(client, "operations/-1"), status=404)
+
+    op2 = Operation.from_response(client, client.get("operations/2").successful())
+
+    assert op2.api_path is not None
+    assert op2.relative_id is not None
+    assert op2.resource_id is not None
+    assert op2.type == "NOOP"
+    assert op2.description is not None
+    assert op2.status is not None
+    assert op2.state == "SUCCEEDED"
+    assert op2.succeeded
+
+    op2a = op2.apply_options(asynchronous=True)
+    assert op2a.succeeded
+
+    op2w = op2a.wait()
+    assert op2w.succeeded
+
+    with pytest.raises(HTTPError):
+        op2w.poll()