Adds set_config to the ECS supported methods. #188
Conversation
Added heirarcical command execution.
Added tests for the set_config functionality.
|
Test PASSed. |
| var changeSet = this.changeSet[key]; | ||
| var length = changeSet.commands.push(command); | ||
| // Add next function to execute to previous next() command. | ||
| changeSet.commands[length - 2].next = this._execute.bind(this, command); |
There was a problem hiding this comment.
Can you flesh this out a little bit more?
There was a problem hiding this comment.
I'm guessing you mean flesh out the comment more...
There was a problem hiding this comment.
Yeah I agree, it would be nice at this point to have a documentation on how you intend to design the changeSet data structure.
For example, does it allow to add a command to a nested key?
E.g. suppose you have the following lazy actions:
- add a new machine;
- create an LXC container inside the machine in 1;
- add a unit to the LXC in 2.
The execution order is obviously important here, and we two nesting levels.
Here is another example:
- add a unit to a machine;
- remove that unit.
This should result in a no-op when the queryset is committed. Will this kind of logic be handled here?
Another example (sorry, I write too much):
- create a new service;
- change settings for that service;
- change settings for that service again.
In this case, I guess the more convenient thing to do is ignoring 2. With the current approach, it looks like we would execute both 2 and 3.
I'd like to knwo what do you think about the following.
What about having a single addCommand(changeSet, key, command) method allowing for nested keys? e.g. "machine-new-1.lxc-new-1.django/2"? If this is crazy, please ignore me.
Another question: it seems you designed commands to be a linked-list-like array. I am ok with that if we are sure we can easily traverse the tree and modify/remove existing keys. Right now it seems the only traversing enabled automatically executes the next command, and this feels dangerous.
There was a problem hiding this comment.
As per our hangout the underlying structure will change to a list which will then have convenience methods to parse and execute based on internal record references to their hierarchy.
|
This branch is nice Jeff. |
|
Landing as per our hangout discussion with the data structure changes to be coming in the follow-up branches. Thanks for the reviews! |
|
Build failed: Attempt to land pull request failed |
|
Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge |
This branch adds hierarchical command functionality to the ECS along with the set_config env method.
__To QA__
- Open the GUI using the `ecs` flag.
- Drag the liferay from the charmbrowser and click the deploy button once.
- Open the console and run `app.ecs.changeSet` and make note of the id (service-xxx).
- Run the following substituting the id with the appropriate one `app.ecs.setConfig('service-xxx', {'http-port': "80802"}, null, {'http-port': "8080"}, function() {});`.
- Run `app.ecs.changeSet` and there should now be two entries in the 'commands' array the second of which should have the method property set to 'set_config'.
This branch adds hierarchical command functionality to the ECS along with the set_config env method.
To QA
ecsflag.app.ecs.changeSetand make note of the id (service-xxx).app.ecs.setConfig('service-xxx', {'http-port': "80802"}, null, {'http-port': "8080"}, function() {});.app.ecs.changeSetand there should now be two entries in the 'commands' array the second of which should have the method property set to 'set_config'.