RexOps / Rex

Rex, the friendly automation framework
https://www.rexify.org
717 stars 223 forks source link

-H should accept a comma separated list of servers #648

Open kablamo opened 9 years ago

kablamo commented 9 years ago

-H should accept a comma separated list of servers

rex -H 'host1,host2,host3' task

Is it ok if I submit a patch that does this?

ferki commented 9 years ago

@kablamo: it might be because it's early morning for me, but I can't see it yet how that would add real value on top of the existing methods/notations.

What advantages would it have over the already supported space-separated list of servers? rex -H 'host1 host2 host3' task

Or over the various range notations?

rex -H host[1..3] task
rex -H host[1,2,3] task

Can you give us an example use case which cannot be solved with the current set of functionality, but solvable with comma-separated hosts?

krimdomu commented 9 years ago

@ferki

i think many people just uses "," because this is often common way to seperate things. I've also seen in projects this failure some times and people asked me why this is not working. So i think it doesn't hurt to add this. But we must take care to not destroy the range notations.

ferki commented 9 years ago

@krimdomu: yeah, I know and agree that it's likely to be accepted (whoops, I forgot that part in my first comment), I was just genuinely interested in the reasons (partly since I felt the word should was emphasized without real explanation ^^).

kablamo commented 9 years ago

@ferki i'm happy with spaced separated lists of servers. I just didn't know about it. Somehow I missed that when reading the code. Perhaps @krimdomu has a point since I didn't think to try it. Or maybe documentation would fix this problem. Anyway sorry for the noise.

ferki commented 9 years ago

@kablamo: no worries, it's not being considered as noise at all ;)

The space separated notation for -H is on the landing page on rexify.org, though we don't seem to have detailed documentation yet on the CLI options (other than the output of rex -h, which also only hints about this case). So you're totally right about that the documentation could be improved.

Second, if someone doesn't know the possibilities/notation for whatever reason, I guess it just comes naturally to try to provide a comma-separated list of hosts (like you did :). So after some thinking and discussion, we can see the advantage of having it.

Implementation-wise it should be ensured that this does not break other notations, especially the comma that can be used in the range operator (host[1,2,4,7]). I believe we already have pretty good test coverage for hostname parsing (in t/commands/evaluate_hostnames.t), so all in all: feel free to add tests for it and implement it, we're ready to help it to get into core! :)

(ps.: my point was more like about that I thought it's easier to get the documentation right once, than doing the docs, plus implementing, plus supporting yet another notation. Of course, I can be wrong. Like comma-separation is a natural way for listing stuff. Let's have it.)

kablamo commented 9 years ago

@ferki yeah i can see that commas would be tricky to support given the host[1,2,3,4] feature. Better docs is probably the way to go.

mrmuskrat commented 1 year ago

On the rexify.org main page is an example with the quoted hostname however it doesn't contain spaces so it's easy to miss.

The rex command help mentions -H takes space delimited hostnames but there isn't an example so it's easy to miss the need for quotes.

This command line example will execute uptime on all the given hosts (frontend01, frontend02, ...): $ rex -H "frontend[01..05]" -e "say run 'uptime'"

I would suggest adding something like this as a follow up to the existing example.

This command line example will execute uptime on all the given hosts (foo, bar, baz): $ rex -H "foo bar baz" -e "say run 'uptime'"

ferki commented 1 year ago

Thanks for the suggestion, @mrmuskrat :+1: The matters related to the website belong to the RexOps/rexify-website repo, so I created a new issue there from your comment using GitHub's "Reference in new issue" option.