Repository: planetlabs/planet-client-python
Pull request #685 (2022-09-01 20:54:14)

Enable redirect following and avoid treating 302s as errors
Towards resolving #678.

Without these changes, an attempt to download an order with 0.23 produces

```
Error:
```

With these changes, I'm able to download an order.


Patch:
diff --git a/CHANGES.txt b/CHANGES.txt
index ee87aaf..2c25dfd 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,3 +1,12 @@
+2.0a3 (TBD)
+
+- Help for planet-subscriptions-describe and update commands has been
+  corrected, resolving the issue reported in #658.
+- Subscriptions API methods communicating with particular subscription
+  endpoints (cancel_subscription, update_subscription, get_subscription, and
+  get_results) were broken due to missing 'f' prefixes (#687). The prefixes
+  have been added.
+
 2.0a2 (2022-07-04)
 
 - Upgrade httpx to 0.23, change how quota exceptions are caught
diff --git a/docs/cli-tutorial.md b/docs/cli-tutorial.md
index d296fbb..5a4fcb7 100644
--- a/docs/cli-tutorial.md
+++ b/docs/cli-tutorial.md
@@ -334,8 +334,10 @@ of at least one being corrupted in creases
 Now we'll dive into the variety of ways to customize your order. These can all be
 combined with all the commands listed above.
 
-We'll work with a geojson saved online. You should download the 
-[geometry](data/geometry.geojson). 
+We'll work with a geojson that is already saved. You should download the 
+[geometry](https://raw.githubusercontent.com/planetlabs/planet-client-python/main/docs/data/geometry.geojson)
+(and you can see it [on github](https://github.com/planetlabs/planet-client-python/blob/main/docs/data/geometry.geojson)
+or it is also stored in the repo in the [data/](data/) directory. 
 
 You can move that geometry to your current directory and use the following command, or
 tweak the geometry.geojson to refer to where you downloaded it.
@@ -407,8 +409,8 @@ planet orders request --item-type PSScene --name "Two Scenes Composited" \
 #### Clip & Composite
 
 To clip and composite you need to specify the clip in the tools (instead of `--clip`), as you can
-not use `--clip` and `--tools` in the same call. You can see the full JSON for the tools in 
-[here](data/tools-clip-composite.json).
+not use `--clip` and `--tools` in the same call. There is not yet CLI calls to generate the `tools.json`,
+so you can just grab the [full tools.json](https://raw.githubusercontent.com/planetlabs/planet-client-python/main/docs/data/tools-clip-composite.json)
 
 ```console
 planet orders request --item-type PSScene --name "Two Scenes Clipped and Composited" \
@@ -418,8 +420,8 @@ planet orders request --item-type PSScene --name "Two Scenes Clipped and Composi
 One cool little trick is that you can even stream in the JSON directly with `curl`, piping it into the request:
 
 ```console
-curl -s https://gist.githubusercontent.com/cholmes/378d050a263ae433ddbbb91c3439994b/raw/ebcaa54cacdc6f696a0506705785f8ff8dae9af1/ \
-tools-clip-composite.json | planet orders request --item-type PSScene --name "Streaming Clip & Composite" \
+curl -s https://raw.githubusercontent.com/planetlabs/planet-client-python/main/docs/data/tools-clip-composite.json \
+| planet orders request --item-type PSScene --name "Streaming Clip & Composite" \
  --bundle analytic_sr_udm2 --id 20220605_124027_64_242b,20220605_124025_34_242b --tools - | planet orders create
 ```
 
diff --git a/planet/__version__.py b/planet/__version__.py
index c76ca41..7620c28 100644
--- a/planet/__version__.py
+++ b/planet/__version__.py
@@ -1,1 +1,1 @@
-__version__ = '2.0a2'
+__version__ = '2.0a3.dev0'
diff --git a/planet/cli/subscriptions.py b/planet/cli/subscriptions.py
index 95b7f9e..42a4dad 100644
--- a/planet/cli/subscriptions.py
+++ b/planet/cli/subscriptions.py
@@ -104,7 +104,7 @@ async def cancel_subscription_cmd(ctx, subscription_id, pretty):
 @translate_exceptions
 @coro
 async def update_subscription_cmd(ctx, subscription_id, request, pretty):
-    """Cancels a subscription and prints the API response."""
+    """Updates a subscription and prints the API response."""
     async with CliSession(auth=ctx.obj['AUTH']) as session:
         client = SubscriptionsClient(session)
         sub = await client.update_subscription(subscription_id, request)
@@ -118,14 +118,7 @@ async def update_subscription_cmd(ctx, subscription_id, request, pretty):
 @translate_exceptions
 @coro
 async def describe_subscription_cmd(ctx, subscription_id, pretty):
-    """Cancels a subscription and prints the API response.
-
-    This implementation is only a placeholder. To begin, instead
-    of mocking calls to the Subscriptions API, we'll use a
-    collection of fake subscriptions (the all_subs object).
-    After we refactor we will change to mocking the API.
-
-    """
+    """Gets the description of a subscription and prints the API response."""
     async with CliSession(auth=ctx.obj['AUTH']) as session:
         client = SubscriptionsClient(session)
         sub = await client.get_subscription(subscription_id)
diff --git a/planet/clients/subscriptions.py b/planet/clients/subscriptions.py
index 6eb870f..ddf35ff 100644
--- a/planet/clients/subscriptions.py
+++ b/planet/clients/subscriptions.py
@@ -119,7 +119,7 @@ class SubscriptionsClient:
             APIError: on an API server error.
             ClientError: on a client error.
         """
-        url = 'https://api.planet.com/subscriptions/v1/{subscription_id}/cancel'
+        url = f'https://api.planet.com/subscriptions/v1/{subscription_id}/cancel'
         req = Request(url, method='POST')
 
         try:
@@ -146,7 +146,7 @@ class SubscriptionsClient:
             APIError: on an API server error.
             ClientError: on a client error.
         """
-        url = 'https://api.planet.com/subscriptions/v1/{subscription_id}'
+        url = f'https://api.planet.com/subscriptions/v1/{subscription_id}'
         req = Request(url, method='PUT', json=request)
 
         try:
@@ -174,7 +174,7 @@ class SubscriptionsClient:
             APIError: on an API server error.
             ClientError: on a client error.
         """
-        url = 'https://api.planet.com/subscriptions/v1/{subscription_id}'
+        url = f'https://api.planet.com/subscriptions/v1/{subscription_id}'
         req = Request(url, method='GET')
 
         try:
@@ -222,7 +222,7 @@ class SubscriptionsClient:
             ITEMS_KEY = 'results'
 
         params = {'status': [val for val in status or {}]}
-        url = 'https://api.planet.com/subscriptions/v1/{subscription_id}/results'
+        url = f'https://api.planet.com/subscriptions/v1/{subscription_id}/results'
         req = Request(url, params=params)
 
         try:
diff --git a/planet/http.py b/planet/http.py
index c9b2cf8..584c271 100644
--- a/planet/http.py
+++ b/planet/http.py
@@ -93,11 +93,12 @@ class BaseSession:
 
     @classmethod
     def _raise_for_status(cls, response):
-        try:
-            response.raise_for_status()
-        except httpx.HTTPStatusError as e:
-            e.response.read()
-            cls._convert_and_raise(e)
+        if response.is_error:
+            try:
+                response.raise_for_status()
+            except httpx.HTTPStatusError as e:
+                e.response.read()
+                cls._convert_and_raise(e)
 
         return
 
@@ -241,7 +242,9 @@ class Session(BaseSession):
 
         LOGGER.info(f'Session read timeout set to {READ_TIMEOUT}.')
         timeout = httpx.Timeout(10.0, read=READ_TIMEOUT)
-        self._client = httpx.AsyncClient(auth=auth, timeout=timeout)
+        self._client = httpx.AsyncClient(auth=auth,
+                                         timeout=timeout,
+                                         follow_redirects=True)
 
         self._client.headers.update({'User-Agent': self._get_user_agent()})
         self._client.headers.update({'X-Planet-App': 'python-sdk'})
@@ -265,11 +268,12 @@ class Session(BaseSession):
 
     @classmethod
     async def _raise_for_status(cls, response):
-        try:
-            response.raise_for_status()
-        except httpx.HTTPStatusError as e:
-            await e.response.aread()
-            cls._convert_and_raise(e)
+        if response.is_error:
+            try:
+                response.raise_for_status()
+            except httpx.HTTPStatusError as e:
+                await e.response.aread()
+                cls._convert_and_raise(e)
 
         return
 


Test patch:
diff --git a/tests/integration/test_orders_api.py b/tests/integration/test_orders_api.py
index db4b199..9acd4ee 100644
--- a/tests/integration/test_orders_api.py
+++ b/tests/integration/test_orders_api.py
@@ -33,6 +33,7 @@ from planet.clients.orders import OrderStates
 TEST_URL = 'http://www.MockNotRealURL.com/api/path'
 TEST_BULK_CANCEL_URL = f'{TEST_URL}/bulk/orders/v2/cancel'
 TEST_DOWNLOAD_URL = f'{TEST_URL}/download'
+TEST_DOWNLOAD_ACTUAL_URL = f'{TEST_URL}/download_actual'
 TEST_ORDERS_URL = f'{TEST_URL}/orders/v2'
 TEST_STATS_URL = f'{TEST_URL}/stats/orders/v2'
 
@@ -540,8 +541,16 @@ async def test_download_asset_md(tmpdir, session):
         'Content-Type': 'application/json',
         'Content-Disposition': 'attachment; filename="metadata.json"'
     }
+
+    mock_redirect = httpx.Response(HTTPStatus.FOUND,
+                                   headers={
+                                       'Location': TEST_DOWNLOAD_ACTUAL_URL,
+                                       'Content-Length': '0'
+                                   })
     mock_resp = httpx.Response(HTTPStatus.OK, json=md_json, headers=md_headers)
-    respx.get(dl_url).return_value = mock_resp
+
+    respx.get(dl_url).return_value = mock_redirect
+    respx.get(TEST_DOWNLOAD_ACTUAL_URL).return_value = mock_resp
 
     cl = OrdersClient(session, base_url=TEST_URL)
     filename = await cl.download_asset(dl_url, directory=str(tmpdir))
diff --git a/tests/integration/test_subscriptions_api.py b/tests/integration/test_subscriptions_api.py
index ffc01ee..a90867b 100644
--- a/tests/integration/test_subscriptions_api.py
+++ b/tests/integration/test_subscriptions_api.py
@@ -71,8 +71,9 @@ api_mock.route(
     side_effect=[Response(200, json=page) for page in result_pages(size=40)])
 
 
-# This mock API returns posted data to the caller. It's used to test
-# methods that rely on POST or PUT.
+# 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.
 def modify_response(request):
     if request.content:
         return Response(200, json=json.loads(request.content))
@@ -80,22 +81,33 @@ def modify_response(request):
         return Response(200)
 
 
-modify_api_mock = respx.mock()
-modify_api_mock.route(
-    M(url__startswith='https://api.planet.com/subscriptions/v1'),
-    M(method__in=['POST', 'PUT'])).mock(side_effect=modify_response)
+create_mock = respx.mock()
+create_mock.route(host='api.planet.com',
+                  path='/subscriptions/v1',
+                  method='POST').mock(side_effect=modify_response)
 
-sub_api_mock = respx.mock()
-sub_api_mock.route(
-    M(url__startswith='https://api.planet.com/subscriptions/v1'),
-    M(method__in=['GET'])).mock(
-        return_value=Response(200,
-                              json={
-                                  'id': '42',
-                                  'name': 'test',
-                                  'delivery': 'yes, please',
-                                  'source': 'test'
-                              }))
+update_mock = respx.mock()
+update_mock.route(host='api.planet.com',
+                  path__regex=r'^/subscriptions/v1/(\w+)',
+                  method='PUT').mock(side_effect=modify_response)
+
+cancel_mock = respx.mock()
+cancel_mock.route(host='api.planet.com',
+                  path__regex=r'^/subscriptions/v1/(\w+)/cancel',
+                  method='POST').mock(side_effect=modify_response)
+
+# Mock the subscription description API endpoint.
+describe_mock = respx.mock()
+describe_mock.route(
+    host='api.planet.com',
+    path__regex=r'^/subscriptions/v1/(\w+)',
+    method='GET').mock(return_value=Response(200,
+                                             json={
+                                                 'id': '42',
+                                                 'name': 'test',
+                                                 'delivery': 'yes, please',
+                                                 'source': 'test'
+                                             }))
 
 
 def result_pages(status=None, size=40):
@@ -179,7 +191,7 @@ async def test_create_subscription_failure():
 
 
 @pytest.mark.asyncio
-@modify_api_mock
+@create_mock
 async def test_create_subscription_success():
     """Subscription is created, description has the expected items."""
     async with Session() as session:
@@ -201,7 +213,7 @@ async def test_cancel_subscription_failure():
 
 
 @pytest.mark.asyncio
-@modify_api_mock
+@cancel_mock
 async def test_cancel_subscription_success():
     """Subscription is canceled, description has the expected items."""
     async with Session() as session:
@@ -220,7 +232,7 @@ async def test_update_subscription_failure():
 
 
 @pytest.mark.asyncio
-@modify_api_mock
+@update_mock
 async def test_update_subscription_success():
     """Subscription is created, description has the expected items."""
     async with Session() as session:
@@ -243,7 +255,7 @@ async def test_get_subscription_failure():
 
 
 @pytest.mark.asyncio
-@sub_api_mock
+@describe_mock
 async def test_get_subscription_success(monkeypatch):
     """Subscription description fetched, has the expected items."""
     async with Session() as session:
diff --git a/tests/integration/test_subscriptions_cli.py b/tests/integration/test_subscriptions_cli.py
index 2d7e66b..718b8df 100644
--- a/tests/integration/test_subscriptions_cli.py
+++ b/tests/integration/test_subscriptions_cli.py
@@ -22,8 +22,10 @@ from planet.cli import cli
 
 from test_subscriptions_api import (api_mock,
                                     failing_api_mock,
-                                    modify_api_mock,
-                                    sub_api_mock,
+                                    create_mock,
+                                    update_mock,
+                                    cancel_mock,
+                                    describe_mock,
                                     res_api_mock)
 
 
@@ -86,7 +88,7 @@ GOOD_SUB_REQUEST = {'name': 'lol', 'delivery': True, 'source': 'wut'}
 @pytest.mark.parametrize('cmd_arg, runner_input',
                          [('-', json.dumps(GOOD_SUB_REQUEST)),
                           (json.dumps(GOOD_SUB_REQUEST), None)])
-@modify_api_mock
+@create_mock
 def test_subscriptions_create_success(cmd_arg, runner_input):
     """Subscriptions creation succeeds with a valid subscription request."""
     # The "-" argument says "read from stdin" and the input keyword
@@ -140,7 +142,7 @@ def test_subscriptions_cancel_failure():
     assert result.exit_code == 1  # failure.
 
 
-@modify_api_mock
+@cancel_mock
 def test_subscriptions_cancel_success():
     """Cancel command succeeds."""
     result = CliRunner().invoke(
@@ -168,7 +170,7 @@ def test_subscriptions_update_failure():
     assert result.exit_code == 1  # failure.
 
 
-@modify_api_mock
+@update_mock
 def test_subscriptions_update_success():
     """Update command succeeds."""
     request = GOOD_SUB_REQUEST.copy()
@@ -200,7 +202,7 @@ def test_subscriptions_describe_failure():
     assert result.exit_code == 1  # failure.
 
 
-@sub_api_mock
+@describe_mock
 def test_subscriptions_describe_success():
     """Describe command succeeds."""
     result = CliRunner().invoke(