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

Fix nxos_ssh LLDP neighbors implementation #872

Closed
wants to merge 6 commits into from

Conversation

jobec
Copy link
Contributor

@jobec jobec commented Nov 30, 2018

Fixes #834

@coveralls
Copy link

coveralls commented Nov 30, 2018

Coverage Status

Coverage decreased (-0.1%) to 80.383% when pulling fbadc25 on jobec:nxos_lldp_fix into e353387 on napalm-automation:develop.

@jobec
Copy link
Contributor Author

jobec commented Nov 30, 2018

Couple of remarks:

  1. There was an inconsistency between the interface names from get_interfaces() and get_lldp_neighbors()/get_lldp_neighbors_detail(). I changed it (and all mock data) to using the canonical interface name. This is what get_interfaces() returns

  2. Because the remote Chassis ID isn't necessarily a MAC address (it depends on what the neighbor implemented as a chassis id type), I removed the turn-chassis-id-into-mac piece of the code.

@ktbyers
Copy link
Contributor

ktbyers commented Nov 30, 2018

I vote we switch the super calls back to the previous format as it simpler, the standard way we do super in napalm, and probably/possibly the more proper pythonic way to do this in Python3.

https://docs.python.org/3/library/functions.html#super

@jobec
Copy link
Contributor Author

jobec commented Dec 1, 2018

Actually, it is the “pythonic” way to do it like this when you want to keep python 2.7 compatibility.

The way it was, is how the python-future package advertises it. But it’s not something I’ve ever seen in other packages.

Anyway, I’ll revert it.

@jobec
Copy link
Contributor Author

jobec commented Dec 3, 2018

The super call code is reverted

@jobec
Copy link
Contributor Author

jobec commented Dec 16, 2018

The merge request became obsolete due to commit 24e140e for PR #863

@jobec jobec closed this Dec 16, 2018
@jobec jobec deleted the nxos_lldp_fix branch December 20, 2018 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants