Implemented get_cdp_neighbors() and modified get_lldp_neighbors() #185
Conversation
napalm_ios/ios.py
Outdated
| @@ -118,7 +120,7 @@ def __init__(self, hostname, username, password, timeout=60, optional_args=None) | |||
| except KeyError: | |||
| pass | |||
| self.global_delay_factor = optional_args.get('global_delay_factor', 1) | |||
| self.port = optional_args.get('port', 22) | |||
| self.port = optional_args.get('port', {'ssh': 22, 'telnet': 23}[self.transport]) | |||
There was a problem hiding this comment.
This and line 79 are in a separate PR...shouldn't them here (just going to cause issues).
napalm_ios/ios.py
Outdated
| device_type = 'cisco_ios' | ||
| if self.transport != 'ssh': | ||
| device_type += '_' + self.transport | ||
| self.device = ConnectHandler(device_type=device_type, |
There was a problem hiding this comment.
As per above telnet support shouldn't be coupled into this PR.
napalm_ios/ios.py
Outdated
| @@ -670,15 +675,61 @@ def get_optics(self): | |||
|
|
|||
| return optics_detail | |||
|
|
|||
| def get_cdp_neighbors(self): | |||
There was a problem hiding this comment.
Should be a private method i.e. _get_cdp_neighbors(self).
There was a problem hiding this comment.
I would agree were it not that it might seem awkward for people running CDP in their network to have to make a call to get_lldp_neighbors().
There was a problem hiding this comment.
NAPALM requires a uniform interface across all drivers...so we would be (very) unlikely to have a custom Cisco only method call...so yes the call would be to get_lldp_neighbors().
Longer term there should maybe be discussions to rename this or to have a secondary name like get_layer2_neighbors.
If they really want to they can call the private method.
|
Also unit tests need to pass...which on quick glance looks like fix the pylama lint issues and make the get_cdp_neighbors method a private method. |
|
@TheKnightCoder thanks for your contribution but the code is not passing CI, needs rebasing and there is a code freeze right now as we are migrating all the drivers to a new repo. |
|
See napalm-automation/napalm#473 This is a post reunification task. The telnet changes have already been implemented. |
Modified get_lldp_neighbors() to call get_cdp_neighbors() if need-by.
Some other code changed - which is from a different branch to allow for telnet instead of SSH as the protocol.