clustershell icon indicating copy to clipboard operation
clustershell copied to clipboard

Allow to create NodeSet with node wildcard support disabled

Open volans- opened this issue 7 years ago • 3 comments

In #349 the wildcard support for nodes was added. I've noticed that the constructor of ParsingEngine has an optional node_wildcard_enable parameter that defaults to True.

Would you consider a PR that allows to instantiate a NodeSet setting the same parameter to False, hence disabling the wildcard support on-demand?

My use case is a bit special, so I will understand if this doesn't match your plan of development.

In my case I'm using ClusterShell as a library, and I gather the list of target hosts not from static groups but from dynamic backends like PuppetDB or OpenStack API. In the query syntax I'm allowing for both NodeSet powerful syntax and a custom globbing that will be translated into a regex for the backend, and of course this is not anymore possible with ClusterShell 1.8 due to the above change.

To give you a practical example I was allowing a user to query for node[1-2]* that would be expanded by NodeSet into 2 nodes with a wildcard inside that will be expanded by the backend. When testing with ClusterShell 1.8 I'm getting a ClusterShell.NodeUtils.GroupResolverSourceError: <default> exception.

P.S. although documented in many places, the Python API->NodeSet page of the documentation doesn't report the new feature, see https://clustershell.readthedocs.io/en/latest/api/NodeSet.html

volans- avatar Oct 26 '17 10:10 volans-

Hi,

I'm in favor of enabling control of this feature for API users.

We just need to find the proper way to do that. I would like to avoid having a lot of option parameters to NodeSet constructor for each current (and future) parsing features.

This makes me think that it could be nice to give control to this feature to the backend. We could add a match upcall to GroupSource where matching will be made by backend instead of list+map and internal matching. What do you think @volans- about that? Does it seems useful for you use case or not?

I think we need to give opportunity to disable this matching anyway... Probably for CLI too...

Escaping? foo[1-2]\* ?

degremont avatar Oct 26 '17 10:10 degremont

@degremont thanks for your quick reply!

I would like to avoid having a lot of option parameters to NodeSet constructor for each current (and future) parsing features.

I agree, it will not scale in the future.

This makes me think that it could be nice to give control to this feature to the backend. We could add a match upcall to GroupSource where matching will be made by backend instead of list+map and internal matching. What do you think @volans- about that? Does it seems useful for you use case or not?

I'm not sure I fully understand, are you thinking of some sort of callback that I could write and pass to ClusterShell that will be called for matching?

I'm asking because my current repro code is just:

from ClusterShell.NodeSet import NodeSet

nodes = NodeSet('node[1-2]*')

Up to ClusterShell 1.7.3 this worked and it would expand into:

>>> list(nodes)
['node1*', 'node2*']

While with 1.8.0 it raises the exception.

My use case if you want here is just to use NodeSet instead of a Python set because of its additional capabilities, I like very much NodeSets 😉. Am I missing something that I should define in terms of ClusterShell groups that might help / be the right thing to do?

Another approach could be to automatically disable the wildcard capability if there is no group defined, although it might not solve a potential use case in which one uses both groups and want to disable this feature for some other reason.

Escaping? foo[1-2]* ?

I had thought about this possibility too, but I wanted to propose a smaller change that impacted only the API and in an optional way. If you decide to go this way that surely works for me, I could just transparently escape those before passing them to the NodeSet.

volans- avatar Oct 26 '17 11:10 volans-

@degremont I've found a solution for my specific use case, I'm instantiating the NodeSet with resolver=RESOLVER_NOGROUP.

I'm leaving this issue open in case you want to use it for tracking purposes to add the capability of disabling just the wildcard expansion, but feel free to close it if you want.

volans- avatar Oct 28 '17 14:10 volans-