Skip to content

Commit

Permalink
Merge pull request #741 from cderici/wait_for_idle_arg_type_check
Browse files Browse the repository at this point in the history
#741

#### Description

Currently there's no check for the `apps` argument for the `wait_for_idle()` method. So it goes through any iterable that's passed to it, including strings. As a result `wait_for_idle(apps="test")` produces something like the following:

```
INFO juju.model:model.py:2645 Waiting for model:
 t (missing)
 e (missing)
 s (missing)
 t (missing)
```

This introduces a check for the argument to be a `List[str]`.

Fixes #732 

#### QA Steps

Unit tests are added, should be a simple QA.

```
tox -e py3 -- tests/unit/test_model.py::TestModelWaitForIdle
```
  • Loading branch information
jujubot authored Oct 13, 2022
2 parents 879232c + d3f72a8 commit 4d18b7d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
6 changes: 6 additions & 0 deletions juju/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -2581,6 +2581,12 @@ async def wait_for_idle(self, apps=None, raise_on_error=True, raise_on_blocked=F
timeout = timedelta(seconds=timeout) if timeout is not None else None
idle_period = timedelta(seconds=idle_period)
start_time = datetime.now()
# Type check against the common error of passing a str for apps
if apps is not None and (not isinstance(apps, list) or
any(not isinstance(o, str)
for o in apps)):
raise JujuError(f'Expected a List[str] for apps, given {apps}')

apps = apps or self.applications
idle_times = {}
last_log_time = None
Expand Down
17 changes: 16 additions & 1 deletion tests/unit/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from juju.model import Model
from juju.application import Application
from juju import jasyncio
from juju.errors import JujuConnectionError
from juju.errors import JujuConnectionError, JujuError


def _make_delta(entity, type_, data=None):
Expand Down Expand Up @@ -289,6 +289,21 @@ async def test_no_args(self):
# no apps so should return right away
await m.wait_for_idle(wait_for_active=True)

@pytest.mark.asyncio
async def test_apps_no_lst(self):
m = Model()
with self.assertRaises(JujuError):
# apps arg has to be a List[str]
await m.wait_for_idle(apps="should-be-list")

with self.assertRaises(JujuError):
# apps arg has to be a List[str]
await m.wait_for_idle(apps=3)

with self.assertRaises(JujuError):
# apps arg has to be a List[str]
await m.wait_for_idle(apps=[3])

@pytest.mark.asyncio
async def test_timeout(self):
m = Model()
Expand Down

0 comments on commit 4d18b7d

Please sign in to comment.