-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mikrotik RouterOS API support #79
Mikrotik RouterOS API support #79
Conversation
new file: /dispatcher/platform_settings/__init__.py new file: dispatcher/platform_settings/api_schemas.py
modified: /tasks/dispatcher/platform_settings/api_schemas.py deleted: /tasks/dispatcher/schema.py
nornir_nautobot/plugins/tasks/dispatcher/mikrotik_routeros_api.py
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
modified: nornir_nautobot/plugins/tasks/dispatcher/mikrotik_routeros_api.py deleted: nornir_nautobot/plugins/tasks/dispatcher/platform_settings/__init__.py deleted: nornir_nautobot/plugins/tasks/dispatcher/platform_settings/api_schemas.py
This comment was marked as outdated.
This comment was marked as outdated.
This reverts commit 6267e22.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
modified: nornir_nautobot/plugins/tasks/dispatcher/mikrotik_routeros_api.py new file: nornir_nautobot/plugins/tasks/dispatcher/platform_settings/__init__.py new file: nornir_nautobot/plugins/tasks/dispatcher/platform_settings/configuration.py
This comment was marked as outdated.
This comment was marked as outdated.
modified: nornir_nautobot/plugins/tasks/dispatcher/cisco_asa.py modified: nornir_nautobot/plugins/tasks/dispatcher/cisco_ios.py modified: nornir_nautobot/plugins/tasks/dispatcher/cisco_ios_xr.py modified: nornir_nautobot/plugins/tasks/dispatcher/cisco_nxos.py modified: nornir_nautobot/plugins/tasks/dispatcher/default.py modified: nornir_nautobot/plugins/tasks/dispatcher/juniper_junos.py modified: nornir_nautobot/plugins/tasks/dispatcher/mikrotik_routeros_api.py deleted: nornir_nautobot/plugins/tasks/dispatcher/platform_settings/__init__.py deleted: nornir_nautobot/plugins/tasks/dispatcher/platform_settings/configuration.py modified: nornir_nautobot/plugins/tasks/dispatcher/ruckus_fastiron.py modified: pyproject.toml
feature: refactor get_config method type modified: nornir_nautobot/plugins/tasks/dispatcher/default.py modified: nornir_nautobot/plugins/tasks/dispatcher/mikrotik_routeros_api.py
modified: nornir_nautobot/plugins/tasks/dispatcher/mikrotik_routeros.py
Changes have been implemented, this is review-ready :: Nornir Driver that provides functionality to handle Mikrotik RouterOS API interactions with v6.x and v7.x:
Module-Level modifications:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not add a dependency that hasn't been updated since before I started here.
pyproject.toml
Outdated
@@ -30,6 +30,7 @@ nornir-jinja2 = "^0" | |||
nornir-netmiko = "^0" | |||
pynautobot = "^1.0.1" | |||
netutils = "^1" | |||
routeros-api = "^0.17.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some concerns on why we are importing this as part of the full dependencies. There are no tags/releases since 2019 on the library and isn't needed in order to handle the calls for other platforms.
Is this something that we should look at going without or having a middle app that does this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, good point, should be optional and sanely error out if not importable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can convert it into an extra dependency, so it is installed only in environments that require it. In any other case would be ignored. Pushing that change.
@itdependsnetworks how are you feeling on this one? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's take the functions/methods that we need from the library with attribution. I'd rather us copy the code in with attribution than to accept a library that hasn't had a release in over 3 years.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's take the functions/methods that we need from the library with attribution. I'd rather us copy the code in with attribution than to accept a library that hasn't had a release in over 3 years.
Nornir Driver that provides functionality to handle Mikrotik RouterOS API interactions with v6.x and v7.x(to be tested):
Maintainers:
Options:
☝️ @ maintainers we need Your input on the one above. We have to store a set of API endpoints (schema.py file.)