Change order of netmiko_options #216
Conversation
Pull Request Test Coverage Report for Build 859
💛 - Coveralls |
|
Fixing the mypy issue. |
|
Everything looks good, the only comment I'd make is that I'd implement the test here, in the https://github.com/nornir-automation/nornir/blob/2.0/tests/core/test_connections.py#L8 Basically the order of parameters is resolved by nornir itself so there is no inherent logic in the drivers for it so, conceptually, the test would fit better there I think. |
|
Okay, let me look at that DummyConnectionPlugin. I hadn't seen that earlier. |
| connection_options = {} | ||
|
|
||
| netmiko_connection_args = parameters | ||
| netmiko_connection_args.update(connection_options) |
There was a problem hiding this comment.
Not critical, but something that crossed my mind:
we could name things in the connection plugin as plugin-agnostic as possible. For example:
napalm_to_netmiko_map -> PLATFORM_MAPPING
netmiko_connection_args -> connection_args
(naming is just an example, not saying it should be named this way)
The benefit would be that if a user wants to implement a custom plugin, he could take literally any existing plugin, do minimal changes and have a new connection plugin.
I don't want to force my ideology here, so I will leave the decision to you.
|
I will redo this once #224 is done. |
|
Closing this one as I think it's obsolete, feel free to reopen otherwise. |
netmiko_options should override inventory arguments.
Add unit tests corresponding to this.