Skip to content

Support bulk subs creation in client and add --bulk to CLI to invoke it #1119

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

Merged
merged 6 commits into from
May 1, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
17 changes: 14 additions & 3 deletions planet/cli/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ async def list_subscriptions_cmd(ctx,

@subscriptions.command(name="create") # type: ignore
@click.argument("request", type=types.JSON())
@click.option(
"--bulk",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm split on whether or not the CLI bulk create path should be an optional flag on the standard create argument as you have implemented here (e.g. create --bulk), or if it should be it's own argument (e.g. bulk,
bulk create, bulk_create, create_bulk, etc).

Including an optional --bulk flag on the standard create command implies that the --bulk flag modifies the behavior of the create command. This isn't really the case though because bulk create has it's own API endpoint POST /subscriptions/v1/bulk. Thinking forward, it might be difficult to enhance and/or modify the bulk create path in the CLI if tightly couple it with the standard create path.

Defining the bulk create path as a distinct argument implies that this is a distinct path, separate from the standard create path. Keeping the paths separate should make it easier to enhance and/or modify the bulk create CLI UX without getting wires crossed with the standard create path.

One more thing to think about is that the payloads for the two paths are slightly different. The standard create path expects a single object of subscription criteria, and the bulk create path expects an array of objects that define subscription criteria. I can see supporting two different request data structures for the same create command, and driven by the --bulk flag being a confusing UX.

Having typed this out and thought about it some more, I am leaning more towards the bulk create path having it's own distinct argument on account of the different endpoints and request data structures.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That all makes sense to me! Update to break into a bulk-create subcommand. Let me know how it looks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! The distinct bulk-create sub-command on the request argument looks great! Could you also remove the --bulk flag option from the create command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! Thought I grabbed that but apparently not. Fixed.

is_flag=True,
help="Bulk create many subscriptions using a feature collection reference.",
)
@click.option(
"--hosting",
type=click.Choice([
Expand Down Expand Up @@ -198,9 +203,15 @@ async def create_subscription_cmd(ctx, request, pretty, **kwargs):
hosting_info = sentinel_hub(collection_id, create_configuration)
request["hosting"] = hosting_info

async with subscriptions_client(ctx) as client:
sub = await client.create_subscription(request)
echo_json(sub, pretty)
if kwargs.get("bulk"):
async with subscriptions_client(ctx) as client:
links = await client.bulk_create_subscriptions([request])
# Bulk create returns just a link to an endpoint to list created subscriptions.
echo_json(links, pretty)
else:
async with subscriptions_client(ctx) as client:
sub = await client.create_subscription(request)
echo_json(sub, pretty)


@subscriptions.command(name='cancel') # type: ignore
Expand Down
30 changes: 29 additions & 1 deletion planet/clients/subscriptions.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Planet Subscriptions API Python client."""

import logging
from typing import Any, AsyncIterator, Awaitable, Dict, Optional, Sequence, TypeVar
from typing import Any, AsyncIterator, Awaitable, Dict, Optional, Sequence, TypeVar, List

from typing_extensions import Literal

Expand Down Expand Up @@ -203,6 +203,34 @@ async def create_subscription(self, request: dict) -> dict:
sub = resp.json()
return sub

async def bulk_create_subscriptions(self, requests: List[dict]) -> Dict:
"""
Create multiple subscriptions in bulk. Currently, the list of requests can only contain one item.

Args:
requests (List[dict]): A list of dictionaries where each dictionary
represents a subscription to be created.

Raises:
APIError: If the API returns an error response.
ClientError: If there is an issue with the client request.

Returns:
The response including a _links key to the list endpoint for use finding the created subscriptions.
"""
try:
url = f'{self._base_url}/bulk'
resp = await self._session.request(
method='POST', url=url, json={'subscriptions': requests})
# 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:
return resp.json()

async def cancel_subscription(self, subscription_id: str) -> None:
"""Cancel a Subscription.

Expand Down
18 changes: 17 additions & 1 deletion planet/sync/subscriptions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Planet Subscriptions API Python client."""

from typing import Any, Dict, Iterator, Optional, Sequence, Union
from typing import Any, Dict, Iterator, Optional, Sequence, Union, List

from typing_extensions import Literal

Expand Down Expand Up @@ -136,6 +136,22 @@ def create_subscription(self, request: Dict) -> Dict:
return self._client._call_sync(
self._client.create_subscription(request))

def bulk_create_subscriptions(self, requests: List[Dict]) -> Dict:
"""Bulk create subscriptions.

Args:
request (List[dict]): list of descriptions of a bulk creation.

Returns:
response including link to list of created subscriptions

Raises:
APIError: on an API server error.
ClientError: on a client error.
"""
return self._client._call_sync(
self._client.bulk_create_subscriptions(requests))

def cancel_subscription(self, subscription_id: str) -> None:
"""Cancel a Subscription.

Expand Down
23 changes: 23 additions & 0 deletions tests/integration/test_subscriptions_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,17 @@ def modify_response(request):
create_mock.route(M(url=TEST_URL),
method='POST').mock(side_effect=modify_response)

bulk_create_mock = respx.mock()
bulk_create_mock.route(
M(url=f'{TEST_URL}/bulk'), method='POST'
).mock(return_value=Response(
200,
json={
'_links': {
'list': f'{TEST_URL}/subscriptions/v1?created={datetime.now().isoformat()}/&geom_ref=pl:features:test_features&name=test-sub'
}
}))

update_mock = respx.mock()
update_mock.route(M(url=f'{TEST_URL}/test'),
method='PUT').mock(side_effect=modify_response)
Expand Down Expand Up @@ -334,6 +345,18 @@ async def test_create_subscription_success():
assert sub['name'] == 'test'


@pytest.mark.anyio
@bulk_create_mock
async def test_bulk_create_subscription_success():
"""Bulk subscription is created, description has the expected items."""
async with Session() as session:
client = SubscriptionsClient(session, base_url=TEST_URL)
resp = await client.bulk_create_subscriptions([{
'name': 'test', 'delivery': 'yes, please', 'source': 'test'
}])
assert '/subscriptions/v1?' in resp['_links']['list']


@create_mock
def test_create_subscription_success_sync():
"""Subscription is created, description has the expected items."""
Expand Down