-
Notifications
You must be signed in to change notification settings - Fork 63
Add aerleon and capirca platform mappers #425
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
Add aerleon and capirca platform mappers #425
Conversation
netutils/lib_mapper.py
Outdated
| NETMIKO_LIB_MAPPER_REVERSE = copy.deepcopy(NETMIKO_LIB_MAPPER) | ||
| NTCTEMPLATES_LIB_MAPPER_REVERSE = copy.deepcopy(NTCTEMPLATES_LIB_MAPPER) | ||
|
|
||
| NETUTILS_LIBMAPPER_MAPPER = { |
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.
What does this offer over "MAIN_LIB_MAPPER"?
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.
It appears to me that main lib mapper is more focused on the end platform, rather than the library that has a platform. I can't do MAIN_LIB_MAPPER['aerleon'] or MAIN_LIB_MAPPER['napalm'] unless I am mistaken?
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.
Any thoughts 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.
ok, should it be "netmiko": list(NETMIKO_LIB_MAPPER.keys()), then?
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.
Thinking about it more, it should just be
from netutils import lib_mapper
getattr(lib_mapper, f"{lib.upper()}_LIB_MAPPER")|
Can we just list all of them so they don't trickle in as much? |
|
Do you mean list all of the platforms supported by Aerleon/Capirca? I didn't add them only because many of them don't have another name to be mapped to, which I am fine with adding as I see in some other mappers, it's the same. |
|
Yea, would be good to know what those are, even if commented out for now. |
|
@scetron let's remove the NETUTILS_LIBMAPPER_MAPPER and add to the dictionary, just to get capirca and aerleon in, we can take the topic of NETUTILS_LIBMAPPER_MAPPER to another issue. |
|
@itdependsnetworks Sounds good. Removed. @qduk Added to dev scripts and generated the new docs. |
qduk
left a comment
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.
Thanks @scetron!
* add capirca/aerleon lib mapper
Added common network platforms supported by Aerleon and Capirca.