From 87abba196c20323eede1f96d13330633e388bc40 Mon Sep 17 00:00:00 2001 From: Adam Weiner Date: Tue, 6 Feb 2024 11:23:47 -0700 Subject: [PATCH 1/2] subscriptions: add support for patch endpoint --- CONTRIBUTING.md | 4 +-- planet/cli/subscriptions.py | 23 ++++++++++++- planet/clients/subscriptions.py | 37 +++++++++++++++++++-- tests/integration/conftest.py | 1 - tests/integration/test_subscriptions_api.py | 24 ++++++++++--- tests/integration/test_subscriptions_cli.py | 35 +++++++++++++++++-- 6 files changed, 110 insertions(+), 14 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f33079d9..20ec8c90 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -33,7 +33,7 @@ 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}` @@ -41,7 +41,7 @@ For example: `release-contributing-691` for [ticket 691](https://github.com/plan ### 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. diff --git a/planet/cli/subscriptions.py b/planet/cli/subscriptions.py index b4f91a47..b3bfe2ba 100644 --- a/planet/cli/subscriptions.py +++ b/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 diff --git a/planet/clients/subscriptions.py b/planet/clients/subscriptions.py index bf92193b..5717f555 100644 --- a/planet/clients/subscriptions.py +++ b/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. diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 03238f8a..b2ad462e 100644 --- a/tests/integration/conftest.py +++ b/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 diff --git a/tests/integration/test_subscriptions_api.py b/tests/integration/test_subscriptions_api.py index 95d9580d..aef5fa8b 100644 --- a/tests/integration/test_subscriptions_api.py +++ b/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 diff --git a/tests/integration/test_subscriptions_cli.py b/tests/integration/test_subscriptions_cli.py index 1e40eb99..2bb69268 100644 --- a/tests/integration/test_subscriptions_cli.py +++ b/tests/integration/test_subscriptions_cli.py @@ -21,12 +21,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 +193,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.""" From bb463dfd7024c4f31fd48f46afac51a4122c6beb Mon Sep 17 00:00:00 2001 From: Adam Weiner Date: Tue, 6 Feb 2024 14:54:42 -0700 Subject: [PATCH 2/2] add new command to cli test comment --- tests/integration/test_subscriptions_cli.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/integration/test_subscriptions_cli.py b/tests/integration/test_subscriptions_cli.py index 2bb69268..3ac230ee 100644 --- a/tests/integration/test_subscriptions_cli.py +++ b/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.