Skip to content
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

Automatic "get_" prefix in napalm_get.py causes issues with NAPALM Getters #83

Open
miguletztim opened this issue Oct 14, 2024 · 4 comments

Comments

@miguletztim
Copy link

The line in nornir_napalm/plugins/tasks/napalm_get.py at Line 42:

getter = g if g.startswith("get_") else "get_{}".format(g)

is causing issues when trying to retrieve data using custom getters that do not follow the "get_" prefix convention.

Problem:

Currently, the code forces any custom getter to follow the "get_" prefix format. If the provided getter name does not start with "get_", the code prepends "get_" to the getter name, which can lead to incorrect or failed data retrieval. This behavior is restrictive and does not account for the possibility of users defining or using getters that do not follow this naming convention.

Example Scenario:

If a custom getter named traceroute (without "get_" prefix) is passed to the task, the current logic will transform it to get_traceroute, causing the task to fail if this method does not exist.

Expected Behavior:

The code should not enforcing the "get_" prefix, or provide a clear option to bypass this transformation when needed.

Proposed Solution:

Modify the code to only use the getter name as provided. The logic could be simplified as:

getter = g

This means users must explicitly pass "get_interfaces", "get_bgp_neighbors", or any other standard getter prefixed with "get_", instead of relying on the function to automatically add it. This would provide clearer control and avoid conflicts.

For reference on standard NAPALM getters, see the Supported NAPALM Getters.

Steps to Reproduce:

  1. Use a custom getter name without a "get_" prefix.
  2. Run the task using nornir_napalm.
  3. Observe the failure or incorrect getter name being used.

Environment:

  • nornir_napalm version: 0.5.0
  • Python version: 3.12.6
@miguletztim
Copy link
Author

I’ve implemented a fix for this issue, but it appears I lack the permission to push the code as a pull request.

@ktbyers
Copy link
Collaborator

ktbyers commented Oct 14, 2024

Probably the simplest solution is just to make a nornir custom task plugin and use that to call the "getters" that you want to call. You can just use the existing nornir_napalm task-plugin as reference and then modify it to meet your needs.

I am reluctant to change this as this "get_" naming convention is a fairly strong convention in napalm (i.e. napalm-ansible would behave the same way IIRC).

@ktbyers
Copy link
Collaborator

ktbyers commented Oct 15, 2024

Let me know if the proposed workaround is viable for you (or if not, why not).

The proposed fix is not really viable as it breaks existing code. Of course, we could do something like add an additional argument, and default it to the current behavior, but I think the above workaround should work.

@dbarrosop
Copy link
Contributor

A traceroute isn't a getter, it is a command that performs an action. This should be implemented via a custom task the same way we have one for ping. That way the parameters required by it are properly specified and documented.

The reason why the getters where done in the current way was because there are too many and in the way majority of cases there is no need for arguments but that is not true for actions like ping or traceroute

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants