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

Issue:
Implement wait interface, rename poll API command
This tickets covers the following changes:
1. Implement `planet orders wait` CLI command
1. Document use cases
1. Rename orders API poll command to wait

### Implement `planet orders wait` CLI command
Implement a new orders cli command, `wait` with the following interface:

```
Usage: planet orders wait [OPTIONS] [ORDER_ID]

  Wait for order to reach the desired state.

  Polls the service for order status and completes when the order has reached
  the desired state. By default, the desired state is any completed state.
  This can be overridden to any order state with the --state option. The
  polling period is one second, this can be overridden with the --period
  option.

  This command outputs the order details at the end of the wait, optionally
  pretty-printed.

Options:
  --state TEXT   Stop waiting when order reaches this state
  --period TEXT  Duration of time for one polling cycle, in seconds.
  --pretty       Format JSON output.
  --help         Show this message and exit.

```

The `wait` command should also use the `--quiet` option (#369) to optionally disable progress bar reporting.

### Document use cases
Document the following use cases:

User Story: As a CLI user I would like to wait for an order to be ready for download so I can send a message to another machine to do the download.

```
$ planet orders wait 65df4eb0-e416-4243-a4d2-38afcf382c30
```

User Story: As a CLI user I would like to wait for an order to be ready for download and then I would like to download the order.

```
$ planet orders wait 65df4eb0-e416-4243-a4d2-38afcf382c30 \
    && planet orders download 65df4eb0-e416-4243-a4d2-38afcf382c30
```

### Rename orders API poll command to wait
To align the python API and CLI interfaces, rename OrdersClient `poll()` to `wait()`

Blocked by #369 


Pull request:
Rename poll to wait, separate wait and download in cli
This PR implements the following:
- rename `OrdersClient.poll()` to `OrdersClient.wait()`
- in `OrdersClient.wait()`, add `max-attempts` parameter and rename `wait` parameter to `delay`.
- `OrdersClient.wait()` now checks if a specified state has been reached **or passed**. This requires introducing the concept of state sequence and is handled with `orders.ORDER_STATE_SEQUENCE` and `orders.OrderStates`
- in `OrdersClient.wait()`, `report` parameter renamed to `callback`. `callback` no longer takes a keyword argument, state is a required positional argument.  Add `update()` function to `reporting.StateReporter` that fits this pattern.
- add new `planet orders wait` cli command and remove polling/waiting from `planet orders download`
- fix bug in `reporting.StateReporter` when bar is disabled
- all exceptions are either `APIException` or `ClientError`

Closes #381 and #352 

Patch:
diff --git a/examples/orders_create_and_download_multiple_orders.py b/examples/orders_create_and_download_multiple_orders.py
index 0b445fb..8bb632f 100644
--- a/examples/orders_create_and_download_multiple_orders.py
+++ b/examples/orders_create_and_download_multiple_orders.py
@@ -62,8 +62,8 @@ async def create_and_download(order_detail, directory, client):
         order = await client.create_order(order_detail)
         reporter.update(state='created', order_id=order.id)
 
-        # poll
-        await client.poll(order.id, report=reporter.update)
+        # wait for completion
+        await client.wait(order.id, report=reporter.update_state)
 
     # download
     await client.download_order(order.id, progress_bar=True)
diff --git a/planet/cli/cmds.py b/planet/cli/cmds.py
index 42ba397..e6c2bfc 100644
--- a/planet/cli/cmds.py
+++ b/planet/cli/cmds.py
@@ -55,11 +55,11 @@ def translate_exceptions(func):
     def wrapper(*args, **kwargs):
         try:
             func(*args, **kwargs)
-        except exceptions.APIException as ex:
-            raise click.ClickException(ex)
         except exceptions.AuthException:
             raise click.ClickException(
                 'Auth information does not exist or is corrupted. Initialize '
                 'with `planet auth init`.')
+        except exceptions.PlanetException as ex:
+            raise click.ClickException(ex)
 
     return wrapper
diff --git a/planet/cli/orders.py b/planet/cli/orders.py
index 4df7956..4caf843 100644
--- a/planet/cli/orders.py
+++ b/planet/cli/orders.py
@@ -20,7 +20,6 @@ import click
 
 import planet
 from planet import OrdersClient, Session  # allow mocking
-
 from .cmds import coro, translate_exceptions
 from .io import echo_json
 
@@ -54,10 +53,9 @@ def orders(ctx, base_url):
 @click.pass_context
 @translate_exceptions
 @coro
-@click.option('-s',
-              '--state',
+@click.option('--state',
               help='Filter orders to given state.',
-              type=click.Choice(planet.clients.orders.ORDERS_STATES,
+              type=click.Choice(planet.clients.orders.ORDER_STATE_SEQUENCE,
                                 case_sensitive=False))
 @click.option('-l',
               '--limit',
@@ -103,6 +101,77 @@ async def cancel(ctx, order_id):
     click.echo('Cancelled')
 
 
+def split_list_arg(ctx, param, value):
+    if value is None:
+        return None
+    elif value == '':
+        # note, this is specifically checking for an empty string
+        click.BadParameter('Entry cannot be an empty string.')
+
+    # split list by ',' and remove whitespace
+    entries = [i.strip() for i in value.split(',')]
+
+    # validate passed entries
+    for e in entries:
+        if not e:
+            raise click.BadParameter('Entry cannot be an empty string.')
+    return entries
+
+
+@orders.command()
+@click.pass_context
+@translate_exceptions
+@coro
+@click.argument('order_id', type=click.UUID)
+@click.option('--delay',
+              type=int,
+              default=5,
+              help='Time (in seconds) between polls.')
+@click.option('--max-attempts',
+              type=int,
+              default=5,
+              help='Maximum number of polls. Set to zero for no limit.')
+@click.option('--quiet',
+              is_flag=True,
+              default=False,
+              help='Disable ANSI control output.')
+@click.option('--state',
+              help='State prior to a final state that will end polling.',
+              type=click.Choice(planet.clients.orders.ORDER_STATE_SEQUENCE,
+                                case_sensitive=False))
+async def wait(ctx, order_id, delay, max_attempts, quiet, state):
+    """Wait until order reaches desired state.
+
+    Reports the state of the order on the last poll.
+
+    This function polls the Orders API to determine the order state, with
+    the specified delay between each polling attempt, until the
+    order reaches a final state or earlier state, if specified.
+    If the maximum number of attempts is reached before polling is
+    complete, an exception is raised. Setting --max-attempts to zero will
+    result in no limit on the number of attempts.
+
+    Setting --delay to zero results in no delay between polling attempts.
+    This will likely result in throttling by the Orders API, which has
+    a rate limit of 10 requests per second. If many orders are being
+    polled asynchronously, consider increasing the delay to avoid
+    throttling.
+
+    By default, polling completes when the order reaches a final state.
+    If --state is specified, polling will complete when the specified earlier
+    state is reached or passed.
+    """
+    async with orders_client(ctx) as cl:
+        with planet.reporting.StateBar(order_id=order_id,
+                                       disable=quiet) as bar:
+            state = await cl.wait(str(order_id),
+                                  state=state,
+                                  delay=delay,
+                                  max_attempts=max_attempts,
+                                  callback=bar.update_state)
+    click.echo(state)
+
+
 @orders.command()
 @click.pass_context
 @translate_exceptions
@@ -112,7 +181,7 @@ async def cancel(ctx, order_id):
               '--quiet',
               is_flag=True,
               default=False,
-              help=('Disable ANSI control output.'))
+              help='Disable ANSI control output.')
 @click.option('-o',
               '--overwrite',
               is_flag=True,
@@ -126,29 +195,12 @@ async def cancel(ctx, order_id):
                               writable=True,
                               file_okay=False))
 async def download(ctx, order_id, quiet, overwrite, dest):
-    '''Download order by order ID.'''
-
-    # Download the user's order
+    """Download order by order ID."""
     async with orders_client(ctx) as cl:
-        with planet.reporting.StateBar(order_id=order_id,
-                                       disable=quiet) as bar:
-            await cl.poll(str(order_id), report=bar.update)
-            _ = await cl.download_order(str(order_id),
-                                        directory=dest,
-                                        overwrite=overwrite,
-                                        progress_bar=not quiet)
-
-
-def split_id_list(ctx, param, value):
-    # split list by ',' and remove whitespace
-    ids = [i.strip() for i in value.split(',')]
-
-    # validate passed ids
-    for iid in ids:
-        if not iid:
-            raise click.BadParameter('id cannot be empty string.')
-
-    return ids
+        await cl.download_order(str(order_id),
+                                directory=dest,
+                                overwrite=overwrite,
+                                progress_bar=not quiet)
 
 
 def read_file_geojson(ctx, param, value):
@@ -184,7 +236,7 @@ def read_file_json(ctx, param, value):
               'ids',
               help='One or more comma-separated item IDs',
               type=click.STRING,
-              callback=split_id_list,
+              callback=split_list_arg,
               required=True)
 # @click.option('--ids_from_search',
 #               help='Embedded data search')
diff --git a/planet/clients/orders.py b/planet/clients/orders.py
index d70828b..be72a3e 100644
--- a/planet/clients/orders.py
+++ b/planet/clients/orders.py
@@ -31,16 +31,31 @@ ORDERS_PATH = '/orders/v2'
 BULK_PATH = '/bulk/orders/v2'
 
 # Order states https://developers.planet.com/docs/orders/ordering/#order-states
-ORDERS_STATES_COMPLETE = ['success', 'partial', 'cancelled', 'failed']
-ORDERS_STATES_IN_PROGRESS = ['queued', 'running']
-ORDERS_STATES = ORDERS_STATES_IN_PROGRESS + ORDERS_STATES_COMPLETE
+# this is in order of state progression except for final states
+ORDER_STATE_SEQUENCE = \
+    ('queued', 'running', 'failed', 'success', 'partial', 'cancelled')
 
 LOGGER = logging.getLogger(__name__)
 
 
-class OrdersClientException(Exception):
-    """Exceptions thrown by OrdersClient"""
-    pass
+class OrderStates():
+    SEQUENCE = ORDER_STATE_SEQUENCE
+
+    @classmethod
+    def _get_position(cls, state):
+        return cls.SEQUENCE.index(state)
+
+    @classmethod
+    def reached(cls, state, test):
+        return cls._get_position(test) >= cls._get_position(state)
+
+    @classmethod
+    def passed(cls, state, test):
+        return cls._get_position(test) > cls._get_position(state)
+
+    @classmethod
+    def is_final(cls, test):
+        return cls.passed('running', test)
 
 
 class OrdersClient():
@@ -76,14 +91,12 @@ class OrdersClient():
 
     @staticmethod
     def _check_order_id(oid):
-        msg = f'Order id ({oid}) must be a valid UUID string.'
-        if not isinstance(oid, str):
-            raise OrdersClientException(msg)
-
+        """Raises planet.exceptions.ClientError if oid is not a valid UUID"""
         try:
-            uuid.UUID(oid)
-        except ValueError:
-            raise OrdersClientException(msg)
+            uuid.UUID(hex=oid)
+        except (ValueError, AttributeError):
+            msg = f'Order id ({oid}) is not a valid UUID hexadecimal string.'
+            raise exceptions.ClientError(msg)
 
     def _orders_url(self):
         return f'{self._base_url}{ORDERS_PATH}'
@@ -165,7 +178,7 @@ class OrdersClient():
             Order information
 
         Raises:
-            OrdersClientException: If order_id is not valid UUID.
+            planet.exceptions.ClientError: If order_id is not a valid UUID.
             planet.exceptions.APIException: On API error.
         '''
         self._check_order_id(order_id)
@@ -197,7 +210,7 @@ class OrdersClient():
             Empty response
 
         Raises:
-            OrdersClientException: If order_id is not valid UUID.
+            planet.exceptions.ClientError: If order_id is not a valid UUID.
             planet.exceptions.APIException: On API error.
         '''
         self._check_order_id(order_id)
@@ -225,6 +238,8 @@ class OrdersClient():
             Results of the bulk cancel request
 
         Raises:
+            planet.exceptions.ClientError: If an entry in order_ids is not a
+                valid UUID.
             planet.exceptions.APIException: On API error.
         '''
         url = f'{self._base_url}{BULK_PATH}/cancel'
@@ -301,11 +316,21 @@ class OrdersClient():
 
         Raises:
             planet.exceptions.APIException: On API error.
+            planet.exceptions.ClientError: If the order is not in a final
+                state.
         """
         order = await self.get_order(order_id)
+        if not OrderStates.is_final(order.state):
+            raise exceptions.ClientError(
+                'Order cannot be downloaded because the order state '
+                f'({order.state}) is not a final state. '
+                'Consider using wait functionality before '
+                'attempting to download.')
+
         locations = order.locations
         LOGGER.info(
             f'downloading {len(locations)} assets from order {order_id}')
+
         filenames = [
             await self.download_asset(location,
                                       directory=directory,
@@ -315,71 +340,90 @@ class OrdersClient():
         ]
         return filenames
 
-    async def poll(self,
+    async def wait(self,
                    order_id: str,
                    state: str = None,
-                   wait: int = 1,
-                   report=None) -> str:
-        """Poll for order status until order reaches desired state, optionally
-        reporting status.
-
-        By default, the Orders API is polled every 1 second for status updates.
-        The API rate limit for this endpoint is 10 requests per second.
-        If many orders are being polled asynchronously, consider
-        increasing the wait to avoid throttling.
+                   delay: int = 5,
+                   max_attempts: int = 200,
+                   callback: typing.Callable[[str], None] = None) -> str:
+        """Wait until order reaches desired state.
+
+        Returns the state of the order on the last poll.
+
+        This function polls the Orders API to determine the order state, with
+        the specified delay between each polling attempt, until the
+        order reaches a final state, or earlier state, if specified.
+        If the maximum number of attempts is reached before polling is
+        complete, an exception is raised. Setting 'max_attempts' to zero will
+        result in no limit on the number of attempts.
+
+        Setting 'delay' to zero results in no delay between polling attempts.
+        This will likely result in throttling by the Orders API, which has
+        a rate limit of 10 requests per second. If many orders are being
+        polled asynchronously, consider increasing the delay to avoid
+        throttling.
+
+        By default, polling completes when the order reaches a final state.
+        If 'state' is given, polling will complete when the specified earlier
+        state is reached or passed.
 
         Example:
             ```python
             from planet.reporting import StateBar
 
             with StateBar() as bar:
-                await poll(order_id, report=bar.update)
+                await wait(order_id, callback=bar.update_state)
             ```
 
         Parameters:
-            order_id: The ID of the order
-            state: State to poll until. If multiple, use list. Defaults to
-                any completed state.
-            wait: Time (in seconds) between polls.
-            report: Callback function for progress updates. Invoked with
-                keyword arguments `state` (poll state) and `logger`
-                (callback for logging progress bar status). Recommended
-                value is `reporting.StateBar.update`.
+            order_id: The ID of the order.
+            state: State prior to a final state that will end polling.
+            delay: Time (in seconds) between polls.
+            max_attempts: Maximum number of polls. Set to zero for no limit.
+            callback: Function that handles state progress updates.
 
         Returns
-            Completed state of the order.
+            State of the order.
 
         Raises:
             planet.exceptions.APIException: On API error.
-            OrdersClientException: If order_id is not valid or state is not
-                supported.
+            planet.exceptions.ClientError: If order_id or state is not valid or
+                if the maximum number of attempts is reached before the
+                specified state or a final state is reached.
         """
-        if state:
-            if state not in ORDERS_STATES:
-                raise OrdersClientException(
-                    f'{state} should be one of: {ORDERS_STATES}')
-            states = [state]
-        else:
-            states = ORDERS_STATES_COMPLETE
-
-        completed = False
-        while not completed:
+        if state and state not in ORDER_STATE_SEQUENCE:
+            raise exceptions.ClientError(
+                f'{state} must be one of {ORDER_STATE_SEQUENCE}')
+
+        # loop without end if max_attempts is zero
+        # otherwise, loop until num_attempts reaches max_attempts
+        num_attempts = 0
+        while not max_attempts or num_attempts < max_attempts:
             t = time.time()
 
             order = await self.get_order(order_id)
-            state = order.state
+            current_state = order.state
+
+            LOGGER.debug(state)
+
+            if callback:
+                callback(order.state)
+
+            if OrderStates.is_final(current_state) or \
+                    (state and OrderStates.reached(state, current_state)):
+                break
 
-            if report:
-                report(state=order.state)
-            else:
-                LOGGER.debug(state)
+            sleep_time = max(delay - (time.time() - t), 0)
+            LOGGER.debug(f'sleeping {sleep_time}s')
+            await asyncio.sleep(sleep_time)
 
-            completed = state in states
-            if not completed:
-                sleep_time = max(wait - (time.time() - t), 0)
-                LOGGER.debug(f'sleeping {sleep_time}s')
-                await asyncio.sleep(sleep_time)
-        return state
+            num_attempts += 1
+
+        if max_attempts and num_attempts >= max_attempts:
+            raise exceptions.ClientError(
+                f'Maximum number of attempts ({max_attempts}) reached.')
+
+        return current_state
 
     async def list_orders(
             self,
@@ -398,10 +442,15 @@ class OrdersClient():
 
         Raises:
             planet.exceptions.APIException: On API error.
+            planet.exceptions.ClientError: If state is not valid.
         """
         url = self._orders_url()
+
         if state:
-            self._check_state(state)
+            if state not in ORDER_STATE_SEQUENCE:
+                raise exceptions.ClientError(
+                    f'Order state ({state}) is not a valid state. '
+                    f'Valid states are {ORDER_STATE_SEQUENCE}')
             params = {"state": state}
         else:
             params = None
@@ -418,10 +467,3 @@ class OrdersClient():
         request = self._request(url, 'GET', params=params)
 
         return Orders(request, self._do_request, limit=limit)
-
-    @staticmethod
-    def _check_state(state):
-        if state not in ORDERS_STATES:
-            raise OrdersClientException(
-                f'Order state (\'{state}\') should be one of: '
-                f'{ORDERS_STATES}')
diff --git a/planet/exceptions.py b/planet/exceptions.py
index 41e5983..6e93e04 100644
--- a/planet/exceptions.py
+++ b/planet/exceptions.py
@@ -71,11 +71,16 @@ class InvalidIdentity(APIException):
     pass
 
 
-class RequestCancelled(PlanetException):
-    '''Internal exception when a request is cancelled'''
+class ClientError(PlanetException):
+    """Exceptions thrown client-side"""
     pass
 
 
-class AuthException(PlanetException):
+class AuthException(ClientError):
     '''Exceptions encountered during authentication'''
     pass
+
+
+class RequestCancelled(ClientError):
+    '''Internal exception when a request is cancelled'''
+    pass
diff --git a/planet/reporting.py b/planet/reporting.py
index 587f02d..1d0d678 100644
--- a/planet/reporting.py
+++ b/planet/reporting.py
@@ -83,17 +83,22 @@ class StateBar(ProgressBar):
     def desc(self):
         return f'order {self.order_id}'
 
+    def update_state(self, state: str):
+        """Simple function to be used as a callback for state reporting"""
+        self.update(state=state)
+
     def update(self, state: str = None, order_id: str = None):
         if state:
-            if state != self.state:
-                LOGGER.info('{order_id} state: {state}')
             self.state = state
-            self.bar.postfix[1] = self.state
+            try:
+                self.bar.postfix[1] = self.state
+            except AttributeError:
+                # If the bar is disabled, attempting to access self.bar.postfix
+                # will result in an error. In this case, just skip it.
+                pass
 
         if order_id:
             self.order_id = order_id
             self.bar.set_description_str(self.desc, refresh=False)
 
         self.bar.refresh()
-
-        LOGGER.debug(str(self.bar))


Test patch:
diff --git a/tests/integration/test_orders_api.py b/tests/integration/test_orders_api.py
index 4c5abe1..50dd59c 100644
--- a/tests/integration/test_orders_api.py
+++ b/tests/integration/test_orders_api.py
@@ -18,14 +18,14 @@ import logging
 import math
 import os
 from pathlib import Path
-from unittest.mock import create_autospec
+from unittest.mock import call, create_autospec
 
 import httpx
 import pytest
 import respx
 
-from planet import OrdersClient, clients, exceptions, reporting
-# from planet.clients.orders import BULK_PATH, ORDERS_PATH, STATS_PATH
+from planet import OrdersClient, exceptions, reporting
+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'
@@ -54,6 +54,7 @@ def create_download_mock(downloaded_content, order_description, oid):
 
     def f():
         # Create mock HTTP response
+        order_description['state'] = 'success'
         dl_url = TEST_DOWNLOAD_URL + '/1?token=IAmAToken'
         order_description['_links']['results'] = [
             {
@@ -78,6 +79,18 @@ def create_download_mock(downloaded_content, order_description, oid):
     return f
 
 
+def test_OrderStates_reached():
+    assert not OrderStates.reached('running', 'queued')
+    assert OrderStates.reached('running', 'running')
+    assert OrderStates.reached('running', 'failed')
+
+
+def test_OrderStates_passed():
+    assert not OrderStates.reached('running', 'queued')
+    assert not OrderStates.passed('running', 'running')
+    assert OrderStates.passed('running', 'success')
+
+
 @respx.mock
 @pytest.mark.asyncio
 async def test_list_orders_basic(order_descriptions, session):
@@ -134,8 +147,8 @@ async def test_list_orders_state(order_descriptions, session):
 async def test_list_orders_state_invalid_state(session):
     cl = OrdersClient(session, base_url=TEST_URL)
 
-    with pytest.raises(clients.orders.OrdersClientException):
-        _ = await cl.list_orders(state='invalidstate')
+    with pytest.raises(exceptions.ClientError):
+        await cl.list_orders(state='invalidstate')
 
 
 @respx.mock
@@ -263,8 +276,8 @@ async def test_get_order(oid, order_description, session):
 @pytest.mark.asyncio
 async def test_get_order_invalid_id(session):
     cl = OrdersClient(session, base_url=TEST_URL)
-    with pytest.raises(clients.orders.OrdersClientException):
-        _ = await cl.get_order('-')
+    with pytest.raises(exceptions.ClientError):
+        await cl.get_order('-')
 
 
 @respx.mock
@@ -300,8 +313,8 @@ async def test_cancel_order(oid, order_description, session):
 @pytest.mark.asyncio
 async def test_cancel_order_invalid_id(session):
     cl = OrdersClient(session, base_url=TEST_URL)
-    with pytest.raises(clients.orders.OrdersClientException):
-        _ = await cl.cancel_order('invalid_order_id')
+    with pytest.raises(exceptions.ClientError):
+        await cl.cancel_order('invalid_order_id')
 
 
 @respx.mock
@@ -374,7 +387,7 @@ async def test_cancel_orders_by_ids(session, oid):
 @pytest.mark.asyncio
 async def test_cancel_orders_by_ids_invalid_id(session, oid):
     cl = OrdersClient(session, base_url=TEST_URL)
-    with pytest.raises(clients.orders.OrdersClientException):
+    with pytest.raises(exceptions.ClientError):
         _ = await cl.cancel_orders([oid, "invalid_oid"])
 
 
@@ -404,7 +417,7 @@ async def test_cancel_orders_all(session):
 
 @respx.mock
 @pytest.mark.asyncio
-async def test_poll(oid, order_description, session):
+async def test_wait_default(oid, order_description, session):
     get_url = f'{TEST_ORDERS_URL}/{oid}'
 
     order_description2 = copy.deepcopy(order_description)
@@ -412,7 +425,27 @@ async def test_poll(oid, order_description, session):
     order_description3 = copy.deepcopy(order_description)
     order_description3['state'] = 'success'
 
+    route = respx.get(get_url)
+    route.side_effect = [
+        httpx.Response(HTTPStatus.OK, json=order_description),
+        httpx.Response(HTTPStatus.OK, json=order_description2),
+        httpx.Response(HTTPStatus.OK, json=order_description3)
+    ]
+
     cl = OrdersClient(session, base_url=TEST_URL)
+    state = await cl.wait(oid, delay=0)
+    assert state == 'success'
+
+
+@respx.mock
+@pytest.mark.asyncio
+async def test_wait_callback(oid, order_description, session):
+    get_url = f'{TEST_ORDERS_URL}/{oid}'
+
+    order_description2 = copy.deepcopy(order_description)
+    order_description2['state'] = 'running'
+    order_description3 = copy.deepcopy(order_description)
+    order_description3['state'] = 'success'
 
     route = respx.get(get_url)
     route.side_effect = [
@@ -422,14 +455,25 @@ async def test_poll(oid, order_description, session):
     ]
 
     mock_bar = create_autospec(reporting.StateBar)
-    mock_report = mock_bar.update
-    state = await cl.poll(oid, wait=0, report=mock_report)
-    assert state == 'success'
+    mock_callback = mock_bar.update_state
+
+    cl = OrdersClient(session, base_url=TEST_URL)
+    await cl.wait(oid, delay=0, callback=mock_callback)
+
+    # check state was sent to callback as expected
+    expected = [call(s) for s in ['queued', 'running', 'success']]
+    mock_callback.assert_has_calls(expected)
+
+
+@respx.mock
+@pytest.mark.asyncio
+async def test_wait_state(oid, order_description, session):
+    get_url = f'{TEST_ORDERS_URL}/{oid}'
 
-    # check state was reported as expected
-    assert mock_report.call_count == 3
-    states = [c[2]['state'] for c in mock_report.mock_calls]
-    assert ['queued', 'running', 'success'] == states
+    order_description2 = copy.deepcopy(order_description)
+    order_description2['state'] = 'running'
+    order_description3 = copy.deepcopy(order_description)
+    order_description3['state'] = 'success'
 
     route = respx.get(get_url)
     route.side_effect = [
@@ -437,22 +481,55 @@ async def test_poll(oid, order_description, session):
         httpx.Response(HTTPStatus.OK, json=order_description2),
         httpx.Response(HTTPStatus.OK, json=order_description3)
     ]
-    state = await cl.poll(oid, state='running', wait=0)
+
+    cl = OrdersClient(session, base_url=TEST_URL)
+    state = await cl.wait(oid, state='running', delay=0)
     assert state == 'running'
 
 
+@respx.mock
+@pytest.mark.asyncio
+async def test_wait_max_attempts_enabled(oid, order_description, session):
+    get_url = f'{TEST_ORDERS_URL}/{oid}'
+
+    route = respx.get(get_url)
+    route.side_effect = [
+        httpx.Response(HTTPStatus.OK, json=order_description),
+    ]
+
+    cl = OrdersClient(session, base_url=TEST_URL)
+    with pytest.raises(exceptions.ClientError):
+        await cl.wait(oid, max_attempts=1, delay=0)
+
+
+@respx.mock
+@pytest.mark.asyncio
+async def test_wait_max_attempts_disabled(oid, order_description, session):
+    get_url = f'{TEST_ORDERS_URL}/{oid}'
+
+    route = respx.get(get_url)
+    route.side_effect = [
+        httpx.Response(HTTPStatus.OK, json=order_description),
+    ] * 300
+
+    cl = OrdersClient(session, base_url=TEST_URL)
+    with pytest.raises(RuntimeError):
+        # falls off the end of the mocked responses
+        await cl.wait(oid, max_attempts=0, delay=0)
+
+
 @pytest.mark.asyncio
-async def test_poll_invalid_oid(session):
+async def test_wait_invalid_oid(session):
     cl = OrdersClient(session, base_url=TEST_URL)
-    with pytest.raises(clients.orders.OrdersClientException):
-        _ = await cl.poll("invalid_oid", wait=0)
+    with pytest.raises(exceptions.ClientError):
+        await cl.wait("invalid_oid", delay=0)
 
 
 @pytest.mark.asyncio
-async def test_poll_invalid_state(oid, session):
+async def test_wait_invalid_state(oid, session):
     cl = OrdersClient(session, base_url=TEST_URL)
-    with pytest.raises(clients.orders.OrdersClientException):
-        _ = await cl.poll(oid, state="invalid_state", wait=0)
+    with pytest.raises(exceptions.ClientError):
+        await cl.wait(oid, state="invalid_state", delay=0)
 
 
 @respx.mock
@@ -539,6 +616,7 @@ async def test_download_order_success(tmpdir, order_description, oid, session):
     '''
 
     # Mock an HTTP response for download
+    order_description['state'] = 'success'
     dl_url1 = TEST_DOWNLOAD_URL + '/1?token=IAmAToken'
     dl_url2 = TEST_DOWNLOAD_URL + '/2?token=IAmAnotherToken'
     order_description['_links']['results'] = [{
@@ -584,6 +662,25 @@ async def test_download_order_success(tmpdir, order_description, oid, session):
     assert Path(filenames[1]).name == 'm2.json'
 
 
+@respx.mock
+@pytest.mark.asyncio
+async def test_download_order_state(tmpdir, order_description, oid, session):
+    dl_url1 = TEST_DOWNLOAD_URL + '/1?token=IAmAToken'
+    order_description['_links']['results'] = [
+        {
+            'location': dl_url1
+        },
+    ]
+
+    get_url = f'{TEST_ORDERS_URL}/{oid}'
+    mock_resp = httpx.Response(HTTPStatus.OK, json=order_description)
+    respx.get(get_url).return_value = mock_resp
+
+    cl = OrdersClient(session, base_url=TEST_URL)
+    with pytest.raises(exceptions.ClientError):
+        await cl.download_order(oid, directory=str(tmpdir))
+
+
 @respx.mock
 @pytest.mark.asyncio
 async def test_download_order_overwrite_true_preexisting_data(
diff --git a/tests/integration/test_orders_cli.py b/tests/integration/test_orders_cli.py
index 3acd6f3..a638013 100644
--- a/tests/integration/test_orders_cli.py
+++ b/tests/integration/test_orders_cli.py
@@ -12,11 +12,13 @@
 # License for the specific language governing permissions and limitations under
 # the License.
 '''Test Orders CLI'''
+import copy
 from http import HTTPStatus
 import json
 from pathlib import Path
 from unittest.mock import Mock
 
+import click
 from click.testing import CliRunner
 import httpx
 import pytest
@@ -42,6 +44,15 @@ def invoke():
     return _invoke
 
 
+def test_split_list_arg_empty_string():
+    with pytest.raises(click.exceptions.BadParameter):
+        cli.orders.split_list_arg(None, None, '')
+
+
+def test_split_list_arg_None():
+    assert cli.orders.split_list_arg(None, None, None) is None
+
+
 @respx.mock
 def test_cli_orders_list_basic(invoke, order_descriptions):
     next_page_url = TEST_ORDERS_URL + '/blob/?page_marker=IAmATest'
@@ -181,11 +192,65 @@ def test_cli_orders_cancel_id_not_found(invoke, oid):
     assert 'Error: A descriptive error message\n' == result.output
 
 
+@respx.mock
+def test_cli_orders_wait_default(invoke, order_description, oid):
+    get_url = f'{TEST_ORDERS_URL}/{oid}'
+
+    order_description2 = copy.deepcopy(order_description)
+    order_description2['state'] = 'success'
+
+    route = respx.get(get_url)
+    route.side_effect = [
+        httpx.Response(HTTPStatus.OK, json=order_description),
+        httpx.Response(HTTPStatus.OK, json=order_description2)
+    ]
+
+    runner = CliRunner()
+    result = invoke(['wait', '--delay', '0', oid], runner=runner)
+    assert not result.exception
+    assert result.output.endswith('success\n')
+
+
+@respx.mock
+def test_cli_orders_wait_max_attempts(invoke, order_description, oid):
+    get_url = f'{TEST_ORDERS_URL}/{oid}'
+
+    order_description2 = copy.deepcopy(order_description)
+    order_description2['state'] = 'running'
+    order_description3 = copy.deepcopy(order_description)
+    order_description3['state'] = 'success'
+
+    route = respx.get(get_url)
+    route.side_effect = [httpx.Response(HTTPStatus.OK, json=order_description)]
+
+    runner = CliRunner()
+    result = invoke(['wait', '--delay', '0', '--max-attempts', '1', oid],
+                    runner=runner)
+    assert result.exception
+    assert result.output.endswith(
+        'Error: Maximum number of attempts (1) reached.\n')
+
+
+@respx.mock
+def test_cli_orders_wait_quiet(invoke, order_description, oid):
+    get_url = f'{TEST_ORDERS_URL}/{oid}'
+
+    order_description['state'] = 'success'
+
+    route = respx.get(get_url)
+    route.side_effect = [httpx.Response(HTTPStatus.OK, json=order_description)]
+
+    runner = CliRunner()
+    result = invoke(['wait', '--delay', '0', '--quiet', oid], runner=runner)
+    assert not result.exception
+    assert result.output == 'success\n'
+
+
 @pytest.fixture
 def mock_download_response(oid, order_description):
 
     def _func():
-        # Mock an HTTP response for polling and download
+        # Mock an HTTP response for download
         order_description['state'] = 'success'
         dl_url1 = TEST_DOWNLOAD_URL + '/1?token=IAmAToken'
         dl_url2 = TEST_DOWNLOAD_URL + '/2?token=IAmAnotherToken'
@@ -223,7 +288,7 @@ def mock_download_response(oid, order_description):
 
 
 @respx.mock
-def test_cli_orders_download(invoke, mock_download_response, oid):
+def test_cli_orders_download_default(invoke, mock_download_response, oid):
     mock_download_response()
 
     runner = CliRunner()
@@ -231,8 +296,8 @@ def test_cli_orders_download(invoke, mock_download_response, oid):
         result = invoke(['download', oid], runner=runner)
         assert not result.exception
 
-        # no message, output is only progress reporting
-        assert result.output.startswith('\r00:00 - order')
+        # basic check of progress reporting
+        assert 'm1.json' in result.output
 
         # Check that the files were downloaded and have the correct contents
         f1_path = Path(folder) / 'm1.json'
@@ -282,20 +347,29 @@ def test_cli_orders_download_overwrite(invoke,
         assert json.load(open(filepath)) == {'key': 'value'}
 
 
-@pytest.mark.skip(
-    'https://github.com/planetlabs/planet-client-python/issues/352')  # noqa
 @respx.mock
 def test_cli_orders_download_quiet(invoke, mock_download_response, oid):
     mock_download_response()
 
     runner = CliRunner()
     with runner.isolated_filesystem():
-        result = invoke(['download', '-q', oid], runner=runner)
+        result = invoke(['download', '--quiet', oid], runner=runner)
         assert not result.exception
 
-        # no progress reporting, just the message
-        message = 'Downloaded 2 files.\n'
-        assert message == result.output
+
+@respx.mock
+def test_cli_orders_download_state(invoke, order_description, oid):
+    get_url = f'{TEST_ORDERS_URL}/{oid}'
+
+    order_description['state'] = 'running'
+    mock_resp = httpx.Response(HTTPStatus.OK, json=order_description)
+    respx.get(get_url).return_value = mock_resp
+
+    runner = CliRunner()
+    result = invoke(['download', oid], runner=runner)
+
+    assert result.exception
+    assert 'order state (running) is not a final state.' in result.output
 
 
 @pytest.mark.parametrize(
@@ -356,10 +430,10 @@ def test_cli_orders_create_id_empty(invoke):
         '--bundle',
         'analytic',
         '--item-type',
-        'invalid'
+        'PSOrthoTile'
     ])
     assert result.exit_code
-    assert 'id cannot be empty string.' in result.output
+    assert 'Entry cannot be an empty string.' in result.output
 
 
 @respx.mock
diff --git a/tests/unit/test_reporting.py b/tests/unit/test_reporting.py
index f5feec4..5a37fee 100644
--- a/tests/unit/test_reporting.py
+++ b/tests/unit/test_reporting.py
@@ -19,7 +19,7 @@ from planet import reporting
 LOGGER = logging.getLogger(__name__)
 
 
-def test_StateBar_enabled():
+def test_StateBar___init___default():
     with reporting.StateBar() as bar:
         expected_init = '..:.. - order  - state: '
         assert (re.fullmatch(expected_init, str(bar)))
@@ -29,6 +29,29 @@ def test_StateBar_enabled():
         assert (re.fullmatch(expected_update, str(bar)))
 
 
-def test_StateBar_disabled():
+def test_StateBar___init___stateandorder():
+    with reporting.StateBar(order_id='1', state='init') as bar:
+        expected_init = '..:.. - order 1 - state: init'
+        assert (re.fullmatch(expected_init, str(bar)))
+
+
+def test_StateBar___init___disabled():
     with reporting.StateBar(disable=True) as bar:
         assert bar.bar.disable
+
+        # just make sure this doesn't error out
+        bar.update(order_id='1', state='init')
+
+
+def test_StateBar_update():
+    with reporting.StateBar() as bar:
+        expected_update = '..:.. - order 1 - state: init'
+        bar.update(order_id='1', state='init')
+        assert (re.fullmatch(expected_update, str(bar)))
+
+
+def test_StateBar_update_state():
+    with reporting.StateBar() as bar:
+        expected_update = '..:.. - order  - state: init'
+        bar.update_state('init')
+        assert (re.fullmatch(expected_update, str(bar)))