alces-software / adminware

A sandbox CLI for running commands remotely across nodes
1 stars 0 forks source link

Allow tools to be located within multiple namespaces #48

Closed WilliamMcCumstie closed 6 years ago

WilliamMcCumstie commented 6 years ago

The batch run and open commands operate as plugin systems. They read their respective commands from /var/lib/adminware/tools/{batch,open}/<namespace>/config.yaml. For the first pass, commands can only be within a single namespace. This is done within the action module using ClickGlob.

Eventually multiple namespaces allowing commands such as: admin batch run --node node1 random-namespace restart-node From: /v/l/a/tools/batch/random-namespace/restart-node/config.yaml

This should be implemented by extending the action.ClickGlob.command(click_group, namespace) function. This function is a decorator that takes a click_group and namespace. The click_group input is the command group that the actions will be added to (using: click_group.command). The namespace is used to generate the following glob: /v/l/a/t/<namespace>/*/config.yaml. The returned function decorator can then be used to wrap the actual command (see: https://github.com/alces-software/adminware/blob/6ff15d2c7080c7459efd3842f92909d8606ecbcf/src/commands/batch.py#L104).

After adding the above actions, a secondary glob for sub-directories could be used (/v/l/a/t/<namespace>/*/) and then extract the new directory: sub_namespace. A new command group should then be created using: new_group = click_group.group(sub_namespace). This should allow ClickGlob to be called recursively to add all the subcommands.

WilliamMcCumstie commented 6 years ago

There are a few potential issues with the above methodology, the first being naming conflicts between commands and groups. For example:

/v/l/a/t/batch/run/config.yaml
/v/l/a/t/batch/run/verbose/config.yaml

The above situation will generate both a command and a group called run. This isn't possible in the CLI and one of the two will either win (or fail miserable). Something logical needs to happen in the above situation.

PS: Issues with scp directories Their is a known issue with syncing of directories when running commands (#49). This is separate to the above issue and will be resolved independently.

WilliamMcCumstie commented 6 years ago

There will be another issue determining the namespace name. Currently the models.batch.Batch object is acting as the abstraction to the config.yaml files. A Batch object is created for each action and is passed into the decorated function from action.ClickGlob.command. NOTE: Batch is always passed in, even for open commands.

There is a Batch.__name__() function (I am aware the function name breaks python's style guide, feel free to fix this). It is meant to return the name of the action as typed on the command line such as:

/v/l/a/t/batch/example/config.yaml => Batch.__name__() = example
/v/l/a/t/batch/some/other/example/config.yaml => Batch.__name__() = some other example

This will not work with the recursion and will need to be updated.

WilliamMcCumstie commented 6 years ago

Please note: The models have changed slight with the introduction of Config. Each Batch model corresponds to a Config object. The ClickGlob.command now yields a Config object to the command instead of Batch

DavidMarchant commented 6 years ago

Ok so just a couple of preliminary questions before starting this: a) is this intended to work for arbitrarily-deeply-nested directories? b) in the case of a conflict - as mentioned above - how would you like to proceed?

c) would it be desired to run an entire sub-namespace in sequence ala a command family? (just an idea) d) would you like the autocomplete to only give suggestions in the 'currently selected' directory? Or have all commands have an option on the dropdown, no matter how deep they are?

WilliamMcCumstie commented 6 years ago

To answer your questions (to the best of my abilities): a) Yes, it should work with any arbitrary number of namespaces. This can be done iteratively or recursively b) We can assume that a conflict is an unrecoverable error (more on this in a comment to come) c) Be careful auto generating families, as commands could be grouped by flavour (e.g. reset service/power etc). So there might not be a use case every time. Better to wait for a specific use case before building this functionality in. d) The auto-completion should allow user drill down the namespace. This will require defining nested click_groups. From this, the auto-completion should generate the different commands.

mjtko commented 6 years ago

My 2¢:

a) If it's easier just to have two levels for now, I think that'd probably cover most eventualities. Arbitrary number is a nice-to-have, but not critical. b) I'm happy to mandate that only the lowest level is considered a command. i.e. run/ isn't ever a command (even if somebody has inadvertently/specifically put a config.yaml in there. Can't think that this would ever be a problem. c) What @WilliamMcCumstie said. d) What @WilliamMcCumstie said.

@ColonelPanicks and @WilliamMcCumstie may disagree tho!

WilliamMcCumstie commented 6 years ago

In the case of a naming conflict, this is an unrecoverable error. As it will be mostly us adding the commands, there is no need for a specific tool to diagnose issues.

Instead, a useful error message should be given if a conflict has been detected. This should hopefully be raised when running the adminware command alone. It should contain enough information for the "admin" adding the command to diagnose the conflict and resolve it. This will likely mean printing the config paths.

As far as implementation is concerned, doing the conflict detection upfront will likely be easier. However an on the fly check for conflicts might be more robust. Either approach is acceptable