Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Added action support to amulet. #80
Conversation
marcoceppi
reviewed
Jul 21, 2015
| + raw = juju(['action', 'defined', service, '--format', 'json']) | ||
| + return json.loads(raw) | ||
| + | ||
| + def action_do(self, unit, action, **kw): |
marcoceppi
Jul 21, 2015
Owner
**kw doesn't work well in Juju because action parameters can contain - and other symbols Python doesn't like for key names. Instead a parameters dictionary would work better here, see the configure() action for an example
marcoceppi
Jul 21, 2015
Owner
I'd also argue that action should be less a Deployment method and more a Sentry command
id = d.sentry['unit/#'].do(action, config)
d.action_fetch(id)
but I'm not opposed to the current format (and may even just add a wrapper to UnitSentry later as a short cut)
tasdomas
Jul 21, 2015
Member
I guess it makes sense - sorry, can't say I've wrapped my head around the structure of amulet yet.
marcoceppi
Jul 21, 2015
Owner
Not many have, and that's not to say it's "right" by any means, it was a patterned developed several years ago in a vacuum and just happens to persist through to today.
My suggestion is also limited to how actions run currently (only against a given unit) the future, it'll be actions that can be done against units, services, and who knows what else. I'm fine with this method as it is here and would be happy to make the glue code so that my example above works as well.
marcoceppi
reviewed
Jul 21, 2015
| + :param action_id: Id of the action to fetch results for. | ||
| + """ | ||
| + now = time() | ||
| + while time() - now < timeout: |
marcoceppi
Jul 21, 2015
Owner
You can avoid this loop and have it block by doing juju action fetch --wait=0 which will block until the action returns a completed or failed. I think I've over abused the timeout idea in this whole project.
tvansteenburgh
reviewed
Jul 21, 2015
| + elif wait: | ||
| + sleep(5) | ||
| + continue | ||
| + raise ActionException("timeout while waiting for action results") |
tvansteenburgh
Jul 21, 2015
Member
If wait=False and status != 'completed' the first time through the loop, I think we should just return the result rather than raising ActionException, do you agree? Might be good to have a test case for wait=False also.
|
@marcoceppi, @tvansteenburgh - thanks for the reviews! PTAL |
marcoceppi
reviewed
Jul 21, 2015
| + results_id = action_result["Action queued with id"] | ||
| + return results_id | ||
| + | ||
| + def action_fetch(self, action_id, wait=True, timeout=600): |
marcoceppi
Jul 21, 2015
Owner
Having both a wait and a timeout seems odd, I'd just have
if timeout is not None:
cmd += ["--wait", str(timeout)]
That way a mix of options won't produce uncertain output
tasdomas commentedJul 21, 2015
Added action support to amulet.