Bug #144
hosts decorator can not accept a list object as it's argument
| Status: | New | Start: | 02/10/2010 | |
| Priority: | Normal | Due date: | ||
| Assigned to: |  Jeff Forcier |
% Done: | 0% |
|
| Category: | Core | |||
| Target version: | 0.9.2 | |||
Description
For example, doing the following:
host1 = "user@some.host.com"
host2 = "user@another.host.com"
a_list = [host1, host2]
@hosts(a_list)
def cmd1():
run("ls")
Does not work because the hosts decorator function gets a tuple of objects and does not validate those objects. Setting hosts as follows:
@hosts(host1, host2)
or:
env.hosts = a_list
Gets the desired result from example above. However, there are cases where I want to write a series of functions for the “a_list” group of hosts and cases where I only want functions for a single host.
I'm attaching a trivial diff that basically accomplishes what I'm talking about but I think that there are other use cases that could make this more complicated/interesting. For example, extending the existing defined example above:
host3 = "third.host.com"
@hosts(a_list, host3)
def cmd2():
run("uname -a")
It’s mixing types to accomplish a case that seems more than plausible.
It seems like the hosts decorator should:
- take the tuple it’s given
- validate that objects & handle appropriately
- strings get just passed onto the list
- tuples/lists are iterated over and the individual string objects are passed on.
The one thing I'm intentionally leaving out is the possibility of validating the strings as host names / ip addresses. I'm still trying to figure out if there is any type of host validation elsewhere in the code.
History
Updated by Jeff Forcier 169 days ago
I'm not sure how I feel about this — I generally try to avoid ‘morphing’ function signatures except in extreme situations, and I don’t see why the function signature for @hosts should get all warped just to save someone the trouble of doing @hosts(*a_list) (right? that’s all your first example needs to work as-is) or @hosts(*(a_list + [a_host])) (or maybe more correctly, combined = a_list + [a_host] ; @hosts(*combined).)
Feel free to counter-argue, but I think this is needless complexity for no gain — I don’t see anything compelling in your stated use case that cannot be solved as above.
At the very least, I could see updating these decorators so that they can take either a single iterable or an arg-list (your first example), as that is a semi common pattern. Regarding the second example, I absolutely do not see the point of accepting mixed iterable and string arguments and merging them. That feels quite messy to me, and the onus should be on the user to ensure they match the API rather than the API going “yo, just give me anything and I’ll figure it out!” Again, feel free to enlighten me :)
Updated by Erich Heine 169 days ago
FWIW i prefer the * method myself. However I agree completely that the argument mangling is overkill for this case. What about 2 decorators:
@host('user@host:port')
def single_host_target():
stuff()
@hosts(['user1@host1','user2@host2'])
def multi_host_target():
more_stuf()
This mirrors the command line dichotomy of
fab target:host='user@host' target2:hosts='user@host1;user@host2'
In fact I like this the best, because the name hosts implies a sequence, while the name host implies a single host string.
Updated by Jeff Forcier 169 days ago
- Target version set to 0.9.2
I will admit that the plural vs non plural bit has bugged me for a while, so I’ll consider that, sure :)
Also available in: Atom