Skip to content
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

Wait for idle arg type check #741

Merged
merged 2 commits into from
Oct 13, 2022

Conversation

cderici
Copy link
Contributor

@cderici cderici commented Oct 12, 2022

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

Copy link
Contributor

@juanmanuel-tirado juanmanuel-tirado left a comment

Choose a reason for hiding this comment

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

Apart from the problems regarding the testing phase, this LG2M

@cderici
Copy link
Contributor Author

cderici commented Oct 13, 2022

Apart from the problems regarding the testing phase, this LG2M

This CI issue is slightly more than trivial so it'll be fixed in a new seperate PR, already up at #742 .

@cderici
Copy link
Contributor Author

cderici commented Oct 13, 2022

/merge

@jujubot jujubot merged commit 4d18b7d into juju:master Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wait_for_idle should raise if apps is not a list
3 participants