Merge pull request #1020 from planetlabs/subs-patch-1019

Support Subscriptions API `PATCH` requests

Author: adamweiner

CONTRIBUTING.md

@@ -33,15 +33,15 @@ git checkout -b new-branch-name
 
 #### Branch Naming
 
-Please use the following naming convention for development branchs:
+Please use the following naming convention for development branches:
 
 `{up to 3-word summary of topic, separated by a dash)-{ticket number}`
 
 For example: `release-contributing-691` for [ticket 691](https://github.com/planetlabs/planet-client-python/issues/691).
 
 ### Pull Requests
 
-NOTE: Make sure to set the appropriate base branch for PRs. See Development Branch above for appriopriate branch.
+NOTE: Make sure to set the appropriate base branch for PRs. See Development Branch above for appropriate branch.
 
 The Pull Request requirements are included in the pull request template as a list of checkboxes.
 

planet/cli/subscriptions.py

@@ -127,7 +127,7 @@ async def cancel_subscription_cmd(ctx, subscription_id, pretty):
 @translate_exceptions
 @coro
 async def update_subscription_cmd(ctx, subscription_id, request, pretty):
-    """Update a subscription.
+    """Update a subscription via PUT.
 
     Updates a subscription and prints the updated subscription description,
     optionally pretty-printed.
@@ -140,6 +140,27 @@ async def update_subscription_cmd(ctx, subscription_id, request, pretty):
         echo_json(sub, pretty)
 
 
+@subscriptions.command(name='patch')  # type: ignore
+@click.argument('subscription_id')
+@click.argument('request', type=types.JSON())
+@pretty
+@click.pass_context
+@translate_exceptions
+@coro
+async def patch_subscription_cmd(ctx, subscription_id, request, pretty):
+    """Update a subscription via PATCH.
+
+    Updates a subscription and prints the updated subscription description,
+    optionally pretty-printed.
+
+    REQUEST only requires the attributes to be changed. It must be
+    JSON and can be specified a json string, filename, or '-' for stdin.
+    """
+    async with subscriptions_client(ctx) as client:
+        sub = await client.patch_subscription(subscription_id, request)
+        echo_json(sub, pretty)
+
+
 @subscriptions.command(name='get')  # type: ignore
 @click.argument('subscription_id')
 @pretty

planet/clients/subscriptions.py

@@ -160,11 +160,12 @@ async def cancel_subscription(self, subscription_id: str) -> None:
 
     async def update_subscription(self, subscription_id: str,
                                   request: dict) -> dict:
-        """Update (edit) a Subscription.
+        """Update (edit) a Subscription via PUT.
 
         Args
             subscription_id (str): id of the subscription to update.
-            request (dict): subscription content for update.
+            request (dict): subscription content for update, full
+                payload is required.
 
         Returns:
             dict: description of the updated subscription.
@@ -189,6 +190,38 @@ async def update_subscription(self, subscription_id: str,
             sub = resp.json()
             return sub
 
+    async def patch_subscription(self, subscription_id: str,
+                                 request: dict) -> dict:
+        """Update (edit) a Subscription via PATCH.
+
+        Args
+            subscription_id (str): id of the subscription to update.
+            request (dict): subscription content for update, only
+                attributes to update are required.
+
+        Returns:
+            dict: description of the updated subscription.
+
+        Raises:
+            APIError: on an API server error.
+            ClientError: on a client error.
+        """
+        url = f'{self._base_url}/{subscription_id}'
+
+        try:
+            resp = await self._session.request(method='PATCH',
+                                               url=url,
+                                               json=request)
+        # Forward APIError. We don't strictly need this clause, but it
+        # makes our intent clear.
+        except APIError:
+            raise
+        except ClientError:  # pragma: no cover
+            raise
+        else:
+            sub = resp.json()
+            return sub
+
     async def get_subscription(self, subscription_id: str) -> dict:
         """Get a description of a Subscription.
 

tests/integration/conftest.py

@@ -28,7 +28,6 @@ def test_disable_limiter(monkeypatch):
 
 
 @pytest.fixture
-@pytest.mark.anyio
 async def session():
     async with planet.Session() as ps:
         yield ps

tests/integration/test_subscriptions_api.py

@@ -72,8 +72,8 @@ def result_pages(status=None, size=40):
 
 
 # The "creation", "update", and "cancel" mock APIs return submitted
-# data to the caller. They are used to test methods that rely on POST
-# or PUT.
+# data to the caller. They are used to test methods that rely on POST,
+# PATCH, or PUT.
 def modify_response(request):
     if request.content:
         return Response(200, json=json.loads(request.content))
@@ -89,6 +89,10 @@ def modify_response(request):
 update_mock.route(M(url=f'{TEST_URL}/test'),
                   method='PUT').mock(side_effect=modify_response)
 
+patch_mock = respx.mock()
+patch_mock.route(M(url=f'{TEST_URL}/test'),
+                 method='PATCH').mock(side_effect=modify_response)
+
 cancel_mock = respx.mock()
 cancel_mock.route(M(url=f'{TEST_URL}/test/cancel'),
                   method='POST').mock(side_effect=modify_response)
@@ -232,14 +236,24 @@ async def test_update_subscription_failure():
 @pytest.mark.anyio
 @update_mock
 async def test_update_subscription_success():
-    """Subscription is created, description has the expected items."""
+    """Subscription is updated, description has the expected items."""
     async with Session() as session:
         client = SubscriptionsClient(session, base_url=TEST_URL)
         sub = await client.update_subscription(
             "test", {
-                'name': 'test', 'delivery': "no, thanks", 'source': 'test'
+                "name": "test", "delivery": "no, thanks", "source": "test"
             })
-        assert sub['delivery'] == "no, thanks"
+        assert sub["delivery"] == "no, thanks"
+
+
+@pytest.mark.anyio
+@patch_mock
+async def test_patch_subscription_success():
+    """Subscription is patched, description has the expected items."""
+    async with Session() as session:
+        client = SubscriptionsClient(session, base_url=TEST_URL)
+        sub = await client.patch_subscription("test", {"name": "test patch"})
+        assert sub["name"] == "test patch"
 
 
 @pytest.mark.anyio

tests/integration/test_subscriptions_cli.py

@@ -1,13 +1,14 @@
 """Tests of the Subscriptions CLI (aka planet-subscriptions)
 
-There are 6 subscriptions commands:
+There are 7 subscriptions commands:
 
-[x] planet subscriptions list
+[x] planet subscriptions cancel
+[x] planet subscriptions create
 [x] planet subscriptions get
+[x] planet subscriptions list
+[x] planet subscriptions patch
 [x] planet subscriptions results
-[x] planet subscriptions create
 [x] planet subscriptions update
-[x] planet subscriptions cancel
 
 TODO: tests for 3 options of the planet-subscriptions-results command.
 
@@ -21,12 +22,13 @@
 from planet.cli import cli
 
 from test_subscriptions_api import (api_mock,
-                                    failing_api_mock,
-                                    create_mock,
-                                    update_mock,
                                     cancel_mock,
+                                    create_mock,
+                                    failing_api_mock,
                                     get_mock,
+                                    patch_mock,
                                     res_api_mock,
+                                    update_mock,
                                     TEST_URL)
 
 # CliRunner doesn't agree with empty options, so a list of option
@@ -192,6 +194,34 @@ def test_subscriptions_update_success(invoke):
     assert json.loads(result.output)['name'] == 'new_name'
 
 
+@failing_api_mock
+def test_subscriptions_patch_failure(invoke):
+    """Patch command exits gracefully from an API error."""
+    result = invoke(
+        ['patch', 'test', json.dumps(GOOD_SUB_REQUEST)],
+        # Note: catch_exceptions=True (the default) is required if we want
+        # to exercise the "translate_exceptions" decorator and test for
+        # failure.
+        catch_exceptions=True)
+
+    assert result.exit_code == 1  # failure.
+
+
+@patch_mock
+def test_subscriptions_patch_success(invoke):
+    """Patch command succeeds."""
+    request = {'name': 'test patch'}
+    result = invoke(
+        ['patch', 'test', json.dumps(request)],
+        # Note: catch_exceptions=True (the default) is required if we want
+        # to exercise the "translate_exceptions" decorator and test for
+        # failure.
+        catch_exceptions=True)
+
+    assert result.exit_code == 0  # success.
+    assert json.loads(result.output)['name'] == request['name']
+
+
 @failing_api_mock
 def test_subscriptions_get_failure(invoke):
     """Describe command exits gracefully from an API error."""