Create and destroy model per test #43

Merged
merged 7 commits into from Dec 13, 2016

Conversation

Projects
None yet
3 participants
Owner

johnsca commented Dec 9, 2016

This also includes @bcsaller's TUIView improvements.

bcsaller and others added some commits Nov 8, 2016

Create and destroy model per test
Also, add timeout to glitch actions, minor improvements to the UIs,
and fixes to the default matrix.yaml
- await actionf(rule, model, objects, **action)
+ try:
+ await asyncio.wait_for(actionf(rule, model, objects, **action), 30)
@petevg

petevg Dec 12, 2016

Collaborator

After we've defined success and failure better, as per #41, we should make this fail, I think, as timeouts are generally indicative of bugs.

... or maybe we have a "test core" config param, and if that is true, then timeouts lead to failure.

In any case, I'm okay with this making it into the codebase as is for now.

Collaborator

petevg commented Dec 12, 2016

This looks good, and runs well against a local controller on my laptop.

One minor nitpick: it would be nice to have a "model destroyed" message at the very end. As it was, the last message in my log was "destroying model ...", and I thought that it had hung.

LGTM/+1

LGTM thanks
one minor inline
also the disable timeline issue (for now)

matrix/rules.py
+ await self.add_model(context)
+ await self.run_once(context, test)
+ finally:
+ await self.destroy_model(context)
@bcsaller

bcsaller Dec 12, 2016

Contributor

We may want to honor a cmdline flag to bail here before cleaning the model so the developer can check the remaining units in more detail

Owner

johnsca commented Dec 12, 2016

@petevg Technically, the model is not destroyed when that API call returns, it has just sent the request off to the controller. I added a log message to indicate that it returned, at least, but do you think we should actually block until the model is gone to verify that it was successfully removed?

@bcsaller I added a --keep-models flag, and commented out the timeline action for now.

Collaborator

petevg commented Dec 13, 2016

@johnsca Works great. Looks good. +1 (I don't think that we need to block to make sure the model is destroyed for now -- the user can clean up old models if matrix leaves a mess every once in a while.)

@bcsaller bcsaller merged commit 0914e15 into master Dec 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment