Repository: planetlabs/planet-client-python (python)

Issue:
write interface skeleton for subscription api functions
define subscriptions api function interfaces and each function will currently raise `NotImplementedError`

chunks-
create/update:
* create_subscription
* update_subscription

results:
* get_results

cancel/get/list:
* get_subscription
* cancel_subscription
* list_subscriptions

blocks #512 #513 #514 #516 #517 #518



Pull request description:
Move placeholder client to new client/subscriptions.py module.
Also a fix for #574

Resolves #494

Patch:
diff --git a/planet/cli/subscriptions.py b/planet/cli/subscriptions.py
index 113c6d4..077c641 100644
--- a/planet/cli/subscriptions.py
+++ b/planet/cli/subscriptions.py
@@ -1,201 +1,22 @@
 """Subscriptions CLI"""
 
-from contextlib import asynccontextmanager
-import itertools
 import json
-from typing import AsyncIterator, Dict, Set
-import uuid
 
 import click
 
-import planet
 from planet import Session
 from planet.cli.cmds import coro, translate_exceptions
 from planet.cli.io import echo_json
-from planet.exceptions import PlanetError
-
-# Collections of fake subscriptions and results for testing. Tests will
-# monkeypatch these attributes.
-_fake_subs: Dict[str, dict] = {}
-_fake_sub_results: Dict[str, list] = {}
-
-
-class PlaceholderSubscriptionsClient:
-    """A placeholder client.
-
-    This class and its methods are derived from tests of a skeleton
-    Subscriptions CLI.
-
-    """
-
-    async def list_subscriptions(self,
-                                 status: Set[str] = None,
-                                 limit: int = 100) -> AsyncIterator[dict]:
-        """Get Subscriptions.
-
-        Parameters:
-            status
-            limit
-
-        Yields:
-            dict
-
-        """
-        if status:
-            select_subs = (dict(**sub, id=sub_id) for sub_id,
-                           sub in _fake_subs.items()
-                           if sub['status'] in status)
-        else:
-            select_subs = (
-                dict(**sub, id=sub_id) for sub_id, sub in _fake_subs.items())
-
-        filtered_subs = itertools.islice(select_subs, limit)
-
-        for sub in filtered_subs:
-            yield sub
-
-    async def create_subscription(self, request: dict) -> dict:
-        """Create a Subscription.
-
-        Parameters:
-            request
-
-        Returns:
-            dict
-
-        Raises:
-            PlanetError
-
-        """
-        missing_keys = {'name', 'delivery', 'source'} - request.keys()
-        if missing_keys:
-            raise PlanetError(
-                f"Request lacks required members: {missing_keys!r}")
-
-        id = str(uuid.uuid4())
-        _fake_subs[id] = request
-        sub = _fake_subs[id].copy()
-        sub.update(id=id)
-        return sub
-
-    async def cancel_subscription(self, subscription_id: str) -> dict:
-        """Cancel a Subscription.
-
-        Parameters:
-            subscription_id
-
-        Returns:
-            dict
-
-        Raises:
-            PlanetError
-
-        """
-        try:
-            sub = _fake_subs.pop(subscription_id)
-        except KeyError:
-            raise PlanetError(f"No such subscription: {subscription_id!r}")
-
-        sub.update(id=subscription_id)
-        return sub
-
-    async def update_subscription(self, subscription_id: str,
-                                  request: dict) -> dict:
-        """Update (edit) a Subscription.
-
-        Parameters:
-            subscription_id
-
-        Returns:
-            dict
-
-        Raises:
-            PlanetError
-
-        """
-        try:
-            _fake_subs[subscription_id].update(**request)
-            sub = _fake_subs[subscription_id].copy()
-        except KeyError:
-            raise PlanetError(f"No such subscription: {subscription_id!r}")
-
-        sub.update(id=subscription_id)
-        return sub
-
-    async def get_subscription(self, subscription_id: str) -> dict:
-        """Get a description of a Subscription.
-
-        Parameters:
-            subscription_id
-
-        Returns:
-            dict
-
-        Raises:
-            PlanetError
-
-        """
-        try:
-            sub = _fake_subs[subscription_id].copy()
-        except KeyError:
-            raise PlanetError(f"No such subscription: {subscription_id!r}")
-
-        sub.update(id=subscription_id)
-        return sub
-
-    async def list_subscription_results(
-            self,
-            subscription_id: str,
-            status: Set[str] = None,
-            limit: int = 100) -> AsyncIterator[dict]:
-        """Get Results of a Subscription.
-
-        Parameters:
-            subscription_id
-            status
-            limit
-
-        Yields:
-            dict
-
-        Raises:
-            PlanetError
-
-        """
-        try:
-            if status:
-                select_results = (
-                    result for result in _fake_sub_results[subscription_id]
-                    if result['status'] in status)
-            else:
-                select_results = (
-                    result for result in _fake_sub_results[subscription_id])
-
-            filtered_results = itertools.islice(select_results, limit)
-        except KeyError:
-            raise PlanetError(f"No such subscription: {subscription_id!r}")
-
-        for result in filtered_results:
-            yield result
-
-
-@asynccontextmanager
-async def subscriptions_client(ctx):
-    """Create an authenticated client.
-
-    Note that the session is not currently used with the placeholder
-    client.
-
-    """
-    auth = ctx.obj['AUTH']
-    async with Session(auth=auth):
-        yield PlaceholderSubscriptionsClient()
+from planet.clients.subscriptions import (PlaceholderSubscriptionsClient as
+                                          SubscriptionsClient)
 
 
 @click.group()
 @click.pass_context
 def subscriptions(ctx):
-    ctx.obj['AUTH'] = planet.Auth.from_file()
+    # None means that order of precedence is 1) environment variable,
+    # 2) secret file.
+    ctx.obj['AUTH'] = None
 
 
 # We want our command to be known as "list" on the command line but
@@ -219,7 +40,8 @@ def subscriptions(ctx):
 @coro
 async def list_subscriptions_cmd(ctx, status, limit, pretty):
     """Prints a sequence of JSON-encoded Subscription descriptions."""
-    async with subscriptions_client(ctx) as client:
+    async with Session(auth=ctx.obj['AUTH']) as session:
+        client = SubscriptionsClient(session)
         filtered_subs = client.list_subscriptions(status=status, limit=limit)
         async for sub in filtered_subs:
             echo_json(sub, pretty)
@@ -254,7 +76,8 @@ def parse_request(ctx, param, value: str) -> dict:
 @coro
 async def create_subscription_cmd(ctx, request, pretty):
     """Submits a subscription request and prints the API response."""
-    async with subscriptions_client(ctx) as client:
+    async with Session(auth=ctx.obj['AUTH']) as session:
+        client = SubscriptionsClient(session)
         sub = await client.create_subscription(request)
         echo_json(sub, pretty)
 
@@ -267,7 +90,8 @@ async def create_subscription_cmd(ctx, request, pretty):
 @coro
 async def cancel_subscription_cmd(ctx, subscription_id, pretty):
     """Cancels a subscription and prints the API response."""
-    async with subscriptions_client(ctx) as client:
+    async with Session(auth=ctx.obj['AUTH']) as session:
+        client = SubscriptionsClient(session)
         sub = await client.cancel_subscription(subscription_id)
         echo_json(sub, pretty)
 
@@ -281,7 +105,8 @@ async def cancel_subscription_cmd(ctx, subscription_id, pretty):
 @coro
 async def update_subscription_cmd(ctx, subscription_id, request, pretty):
     """Cancels a subscription and prints the API response."""
-    async with subscriptions_client(ctx) as client:
+    async with Session(auth=ctx.obj['AUTH']) as session:
+        client = SubscriptionsClient(session)
         sub = await client.update_subscription(subscription_id, request)
         echo_json(sub, pretty)
 
@@ -301,7 +126,8 @@ async def describe_subscription_cmd(ctx, subscription_id, pretty):
     After we refactor we will change to mocking the API.
 
     """
-    async with subscriptions_client(ctx) as client:
+    async with Session(auth=ctx.obj['AUTH']) as session:
+        client = SubscriptionsClient(session)
         sub = await client.get_subscription(subscription_id)
         echo_json(sub, pretty)
 
@@ -336,9 +162,10 @@ async def list_subscription_results_cmd(ctx,
                                         status,
                                         limit):
     """Gets results of a subscription and prints the API response."""
-    async with subscriptions_client(ctx) as client:
-        filtered_results = client.list_subscription_results(subscription_id,
-                                                            status=status,
-                                                            limit=limit)
+    async with Session(auth=ctx.obj['AUTH']) as session:
+        client = SubscriptionsClient(session)
+        filtered_results = client.get_results(subscription_id,
+                                              status=status,
+                                              limit=limit)
         async for result in filtered_results:
             echo_json(result, pretty)
diff --git a/planet/clients/subscriptions.py b/planet/clients/subscriptions.py
new file mode 100644
index 0000000..2427854
--- /dev/null
+++ b/planet/clients/subscriptions.py
@@ -0,0 +1,214 @@
+"""Planet Subscriptions API Python client."""
+
+import itertools
+from typing import AsyncIterator, Dict, Optional, Set
+import uuid
+
+from planet.exceptions import ClientError
+
+# Collections of fake subscriptions and results for testing. Tests will
+# monkeypatch these attributes.
+_fake_subs: Optional[Dict[str, dict]] = None
+_fake_sub_results: Optional[Dict[str, list]] = None
+
+
+class PlaceholderSubscriptionsClient:
+    """A placeholder client.
+
+    This class and its methods are derived from tests of a skeleton
+    Subscriptions CLI. It is evolving into the real API client.
+
+    """
+
+    def __init__(self, session=None) -> None:
+        self._session = session
+
+    async def list_subscriptions(self,
+                                 status: Set[str] = None,
+                                 limit: int = 100) -> AsyncIterator[dict]:
+        """Get account subscriptions with optional filtering.
+
+        The name of this method is based on the API's method name. This
+        method provides iteration over subcriptions, it does not return
+        a list.
+
+        Args:
+            status (Set[str]): pass subscriptions with status in this
+                set, filter out subscriptions with status not in this
+                set.
+            limit (int): limit the number of subscriptions in the
+                results.
+
+        Yields:
+            dict: a description of a subscription.
+
+        Raises:
+            ClientError
+
+        """
+        # Temporary marker for behavior of module with unpatched state.
+        if _fake_subs is None:
+            raise NotImplementedError
+
+        if status:
+            select_subs = (dict(**sub, id=sub_id) for sub_id,
+                           sub in _fake_subs.items()
+                           if sub['status'] in status)
+        else:
+            select_subs = (
+                dict(**sub, id=sub_id) for sub_id, sub in _fake_subs.items())
+
+        filtered_subs = itertools.islice(select_subs, limit)
+
+        for sub in filtered_subs:
+            yield sub
+
+    async def create_subscription(self, request: dict) -> dict:
+        """Create a Subscription.
+
+        Args:
+            request (dict): description of a subscription.
+
+        Returns:
+            dict: description of created subscription.
+
+        Raises:
+            ClientError
+
+        """
+        # Temporary marker for behavior of module with unpatched state.
+        if _fake_subs is None:
+            raise NotImplementedError
+
+        missing_keys = {'name', 'delivery', 'source'} - request.keys()
+        if missing_keys:
+            raise ClientError(
+                f"Request lacks required members: {missing_keys!r}")
+
+        id = str(uuid.uuid4())
+        _fake_subs[id] = request
+        sub = _fake_subs[id].copy()
+        sub.update(id=id)
+        return sub
+
+    async def cancel_subscription(self, subscription_id: str) -> dict:
+        """Cancel a Subscription.
+
+        Args:
+            subscription_id (str): id of subscription to cancel.
+
+        Returns:
+            dict: description of cancelled subscription.
+
+        Raises:
+            ClientError
+
+        """
+        # Temporary marker for behavior of module with unpatched state.
+        if _fake_subs is None:
+            raise NotImplementedError
+
+        try:
+            sub = _fake_subs.pop(subscription_id)
+        except KeyError:
+            raise ClientError(f"No such subscription: {subscription_id!r}")
+
+        sub.update(id=subscription_id)
+        return sub
+
+    async def update_subscription(self, subscription_id: str,
+                                  request: dict) -> dict:
+        """Update (edit) a Subscription.
+
+        Args
+            subscription_id (str): id of the subscription to update.
+            request (dict): subscription content for update.
+
+        Returns:
+            dict: description of the updated subscription.
+
+        Raises:
+            ClientError
+
+        """
+        # Temporary marker for behavior of module with unpatched state.
+        if _fake_subs is None:
+            raise NotImplementedError
+
+        try:
+            _fake_subs[subscription_id].update(**request)
+            sub = _fake_subs[subscription_id].copy()
+        except KeyError:
+            raise ClientError(f"No such subscription: {subscription_id!r}")
+
+        sub.update(id=subscription_id)
+        return sub
+
+    async def get_subscription(self, subscription_id: str) -> dict:
+        """Get a description of a Subscription.
+
+        Args:
+            subscription_id (str): id of a subscription.
+
+        Returns:
+            dict: description of the subscription.
+
+        Raises:
+            ClientError
+
+        """
+        # Temporary marker for behavior of module with unpatched state.
+        if _fake_subs is None:
+            raise NotImplementedError
+
+        try:
+            sub = _fake_subs[subscription_id].copy()
+        except KeyError:
+            raise ClientError(f"No such subscription: {subscription_id!r}")
+
+        sub.update(id=subscription_id)
+        return sub
+
+    async def get_results(self,
+                          subscription_id: str,
+                          status: Set[str] = None,
+                          limit: int = 100) -> AsyncIterator[dict]:
+        """Get Results of a Subscription.
+
+        The name of this method is based on the API's method name. This
+        method provides iteration over results, it does not get a
+        single result description or return a list of descriptions.
+
+        Args:
+            subscription_id (str): id of a subscription.
+            status (Set[str]): pass result with status in this set,
+                filter out results with status not in this set.
+            limit (int): limit the number of subscriptions in the
+                results.
+
+        Yields:
+            dict: description of a subscription results.
+
+        Raises:
+            ClientError
+
+        """
+        # Temporary marker for behavior of module with unpatched state.
+        if _fake_sub_results is None:
+            raise NotImplementedError
+
+        try:
+            if status:
+                select_results = (
+                    result for result in _fake_sub_results[subscription_id]
+                    if result['status'] in status)
+            else:
+                select_results = (
+                    result for result in _fake_sub_results[subscription_id])
+
+            filtered_results = itertools.islice(select_results, limit)
+        except KeyError:
+            raise ClientError(f"No such subscription: {subscription_id!r}")
+
+        for result in filtered_results:
+            yield result
diff --git a/planet/http.py b/planet/http.py
index cfbf7ad..48eb7ad 100644
--- a/planet/http.py
+++ b/planet/http.py
@@ -119,8 +119,16 @@ class Session(BaseSession):
 
         Parameters:
             auth: Planet server authentication.
+
         """
-        auth = auth or Auth.from_file()
+        if auth is None:
+            # Try getting credentials from environment before checking
+            # in the secret file, this is the conventional order (AWS
+            # CLI, for example.)
+            try:
+                auth = Auth.from_env()
+            except exceptions.PlanetError:
+                auth = Auth.from_file()
 
         self._client = httpx.AsyncClient(auth=auth)
         self._client.headers.update({'User-Agent': self._get_user_agent()})


Test patch:
diff --git a/tests/integration/test_subscriptions_cli.py b/tests/integration/test_subscriptions_cli.py
index 9cbd417..afdea4c 100644
--- a/tests/integration/test_subscriptions_cli.py
+++ b/tests/integration/test_subscriptions_cli.py
@@ -22,7 +22,7 @@ import pytest
 
 from planet.cli import cli
 import planet.cli.subscriptions
-from planet.cli.subscriptions import PlaceholderSubscriptionsClient
+from planet.clients.subscriptions import PlaceholderSubscriptionsClient
 
 
 @pytest.fixture
@@ -58,7 +58,7 @@ def subscription_count():
 def test_subscriptions_list_options(options, expected_count, monkeypatch):
     """Prints the expected sequence of subscriptions."""
 
-    monkeypatch.setattr(planet.cli.subscriptions,
+    monkeypatch.setattr(planet.clients.subscriptions,
                         '_fake_subs',
                         {str(i): {
                             'status': 'created'
@@ -80,7 +80,7 @@ def test_subscriptions_list_options(options, expected_count, monkeypatch):
 def test_subscriptions_create_failure(monkeypatch, subscription_count):
     """An invalid subscription request fails to create a new subscription."""
 
-    monkeypatch.setattr(planet.cli.subscriptions, '_fake_subs', {})
+    monkeypatch.setattr(planet.clients.subscriptions, '_fake_subs', {})
 
     # This subscription request lacks the required "delivery" and
     # "source" members.
@@ -117,7 +117,7 @@ def test_subscriptions_create_success(cmd_arg,
                                       subscription_count):
     """Subscriptions creation succeeds with a valid subscription request."""
 
-    monkeypatch.setattr(planet.cli.subscriptions, '_fake_subs', {})
+    monkeypatch.setattr(planet.clients.subscriptions, '_fake_subs', {})
 
     # The "-" argument says "read from stdin" and the input keyword
     # argument specifies what bytes go to the runner's stdin.
@@ -163,7 +163,7 @@ def test_subscriptions_bad_request(cmd_arg, runner_input, monkeypatch):
 def test_subscriptions_cancel_failure(monkeypatch):
     """Cancel command exits gracefully from an API error."""
 
-    monkeypatch.setattr(planet.cli.subscriptions, '_fake_subs', {})
+    monkeypatch.setattr(planet.clients.subscriptions, '_fake_subs', {})
 
     result = CliRunner().invoke(
         cli.main,
@@ -180,7 +180,7 @@ def test_subscriptions_cancel_failure(monkeypatch):
 def test_subscriptions_cancel_success(monkeypatch, subscription_count):
     """Cancel command succeeds."""
 
-    monkeypatch.setattr(planet.cli.subscriptions,
+    monkeypatch.setattr(planet.clients.subscriptions,
                         '_fake_subs', {'42': GOOD_SUB_REQUEST})
 
     # Let's check the state of the fake API before we try to cancel.
@@ -202,7 +202,7 @@ def test_subscriptions_cancel_success(monkeypatch, subscription_count):
 def test_subscriptions_update_failure(monkeypatch):
     """Update command exits gracefully from an API error."""
 
-    monkeypatch.setattr(planet.cli.subscriptions, '_fake_subs', {})
+    monkeypatch.setattr(planet.clients.subscriptions, '_fake_subs', {})
 
     result = CliRunner().invoke(
         cli.main,
@@ -219,7 +219,7 @@ def test_subscriptions_update_failure(monkeypatch):
 def test_subscriptions_update_success(monkeypatch):
     """Update command succeeds."""
 
-    monkeypatch.setattr(planet.cli.subscriptions,
+    monkeypatch.setattr(planet.clients.subscriptions,
                         '_fake_subs', {'42': GOOD_SUB_REQUEST})
 
     request = GOOD_SUB_REQUEST.copy()
@@ -240,7 +240,7 @@ def test_subscriptions_update_success(monkeypatch):
 def test_subscriptions_describe_failure(monkeypatch):
     """Describe command exits gracefully from an API error."""
 
-    monkeypatch.setattr(planet.cli.subscriptions, '_fake_subs', {})
+    monkeypatch.setattr(planet.clients.subscriptions, '_fake_subs', {})
 
     result = CliRunner().invoke(
         cli.main,
@@ -257,7 +257,7 @@ def test_subscriptions_describe_failure(monkeypatch):
 def test_subscriptions_describe_success(monkeypatch):
     """Describe command succeeds."""
 
-    monkeypatch.setattr(planet.cli.subscriptions,
+    monkeypatch.setattr(planet.clients.subscriptions,
                         '_fake_subs', {'42': GOOD_SUB_REQUEST})
 
     result = CliRunner().invoke(
@@ -275,7 +275,7 @@ def test_subscriptions_describe_success(monkeypatch):
 def test_subscriptions_results_failure(monkeypatch):
     """Results command exits gracefully from an API error."""
 
-    monkeypatch.setattr(planet.cli.subscriptions, '_fake_sub_results', {})
+    monkeypatch.setattr(planet.clients.subscriptions, '_fake_sub_results', {})
 
     result = CliRunner().invoke(
         cli.main,
@@ -299,7 +299,7 @@ def test_subscriptions_results_success(options, expected_count, monkeypatch):
     """Describe command succeeds."""
 
     monkeypatch.setattr(
-        planet.cli.subscriptions,
+        planet.clients.subscriptions,
         '_fake_sub_results',
         {'42': [{
             'id': f'r{i}', 'status': 'created'