FOR DISCUSSION generic format string for parameters

[dbarrosop]

As you can see from this PR some tasks allowed you to do things like:

brigade.run(commands.command,
            command="echo {host.name}")

Problem is that it was quite inconsistent. Some supported it while others didn't. So I started working on a generic solution for this and although it works I really don't like it as I think it's fragile and it's bound to break things eventually. I already had to handle ValueError exception because in one of my tests it was trying to format a junos configuration leading to an error...

So I was thinking in completely remove this functionality and change the pattern above for:

def echo_hostname(task):
    task.run(commands.command,
             command="echo {}".format(host.name))

brigade.run(echo_hostname)

Thoughts? Should we encourage this pattern or should we try to keep the old one (although inconsistent we can add as necessary) or should we try to make this generic mechanism for resilient? Maybe if we pass it to a jinja2 template it will be better? I think I personally prefer the echo_hostname pattern.

[dbarrosop]

I was also thinking we could do something like this:

format_params = ["path", "src", "dst", (other "standard" parameters)]
params = {}
for k, v in self.params.items():
    if k in format_params:
            params[k] = format_string(v, self, **self.host)
    else:
        params[k] = v

[ktbyers]

Yeah, I don't like this.

brigade.run(commands.command,
            command="echo {host.name}")

I would be worried about the issues you referenced in your original discussion i.e. how hard it would be to make it generally reliable.

I am fine with your def echo_hostname(task): solution.

If we are going to do this, then maybe Jinja2, but I would be worried we were headed down the road of some of things I strongly dislike in Ansible.

So the key issue to clarify to people (assuming that I understand it properly) is when would inventory variables be available to use and what is the proper way to create tasks that need to reference inventory variables.

[ktbyers]

I am going to do some more experiments here.

[ktbyers]

Okay, I tested this some more both by putting the format_string directly into netmiko_send_config and by doing this:

def echo_hostname(task):
    return task.run(netmiko_send_config, config_commands=['hostname {}'.format(task.host.name)])

brigade = Brigade(inventory=SimpleInventory("hosts.yml", "groups.yml"), dry_run=True)
b = brigade.filter(group='cisco-ios')

result = b.run(task=netmiko_connection, num_workers=60)
result = b.run(echo_hostname, num_workers=60)
for k, v in result.items():
    print(v.result)

I probably prefer this pattern...but definitely trade-offs both ways.

I might try to experiment some more with it.

[ktbyers]

Can also do this:

brigade = Brigade(inventory=SimpleInventory("hosts.yml", "groups.yml"), dry_run=True)
b = brigade.filter(group='cisco-ios')

b.run(task=netmiko_connection, num_workers=60)
result = b.run(lambda t: t.run(netmiko_send_config, 
                               config_commands=['hostname {}'.format(t.host.name)]))
for k, v in result.items():
    print(v.result)

[dbarrosop]

Yeah, there are many things we can do. However, I am starting to think what I proposed (#53 (comment)) is really convenient without breaking anything like this PR does. In any case, I have been working during the past week in some "real life" examples to illustrate this and other improvements. I will send a PR soon for broader discussion.

[dbarrosop]

I don't like this implementation so closing.