connection_options extra gathering in a merged way

[viktorkertesz]

Rewrote get_connection_options logic in a way that defaults, group and host connection_options.extra parameters will be merged. Priority is defaults -> group1, group2,... -> host
So a host defined extra parameter would override defaults or groups if the key is the same. All distinct parameters will be merged together.

Benefits

We can better segment our connection_options extra parameters. We can define partial extras in defaults, groups and hosts.

Downside of the new logic

If a user relies on that host defined extra will not merge but replace the default/groups, then the user might get additional parameters which might be unexpected.

Example

defaults.yaml:

---
connection_options:
    netmiko:
        extras:
              secret: nornir

hosts.yaml:

host1:
    connection_options:
        netmiko:
            extras:
                  port: 222

Old behavior

This will result in that host1 will NOT have the secret parameter. Host file replace the extras.

New behavior

host1 will have set the port 222 and the secret set as well. Host file will merge the extras.

TODO

Although I completed tests with the new behavior, I did not do production testing. I'm curious if this all would be interesting to merge into the project. If there is a chance, then I'd work on:

• more comprehensive tests with more realistic inventory
• Complete docs on the behavior. (Currently there is no docs at all on this part of the inventory, not even for the original behavior)

[dbarrosop]

I am afraid this change can’t be considered until we decide it’s time for a new major version (nornir 3.x.y). Semver rules, which we adhere to, prohibit making changes to behavior and/or the API without changing the major version.

In any case, before continuing with this PR I’d suggest you to open a github discussion and discuss it with the community. I personally prefer the current behavior but if there is a certain amount of people that would rather see the new behavior we could see if there is anything that can be done here.

[dbarrosop]

I have a question regarding your implementation. Using your example, what happens if I do this?

nr.inventory.defaults.connection_options[“netmiko”].password = “defined-at-runtime”

print(nr.inventory.hosts[“host1”].connection_options[“netmiko”].password

will it return “defined-at-runtime”? Given your PR description I’d assume so but looking at the implementation I am not so sure. Could you clarify the expected behavior and confirm it behaves that way? Maybe adding a test to avoid regressions even.

No need to fix (if there is anything to fix) or add the rest until we discuss the feature though. Just adding the comment for future reference.

[viktorkertesz]

nr.inventory.defaults.connection_options[“netmiko”].password = “defined-at-runtime”

At the moment I modified only the .get_connection_options method behavior. Therefore modifying defaults' or groups' connection_options attribute won't affect the host's. Looking at .get_connection method, it won't re-run .get_connection_options at the moment if it was already initialized. A further patch could re-run it, to get the most recent merge of all defaults' and groups' connection_options. That is ugly and I would rework the current solution similarly to .extended_data() method.
Thanks for raising this problem!

Let me create a discussion topic on this...

[ktbyers]

@viktorkertesz Are you still actively working on this or should this be closed?

[viktorkertesz]

@ktbyers Unfortunately my idea did not catch the community. I stopped working on this since then. But if you still think it would be a nice feature, I try to make it nice with runtime changes in mind.