Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Subscriptions API PATCH requests #1020

Merged
merged 2 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
23 changes: 22 additions & 1 deletion planet/cli/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
37 changes: 35 additions & 2 deletions planet/clients/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.

Expand Down
1 change: 0 additions & 1 deletion tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 19 additions & 5 deletions tests/integration/test_subscriptions_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
35 changes: 32 additions & 3 deletions tests/integration/test_subscriptions_cli.py
adamweiner marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""
Expand Down
Loading