napalm-automation / napalm-ios

Apache License 2.0
31 stars 40 forks source link

Add config syntax support for 'transport=telnet' #159

Closed The-Loeki closed 7 years ago

The-Loeki commented 7 years ago

I just ran into this issue & hacked up this PR :)

Fixes https://github.com/napalm-automation/napalm-ios/issues/132

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.2%) to 70.129% when pulling 3224f96be7181d092f177610e6879c84e6b8a0c6 on The-Loeki:allow-telnet into b69c73b7cc9620567006c2e36212d65044abeb68 on napalm-automation:develop.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+4.5%) to 74.785% when pulling 3224f96be7181d092f177610e6879c84e6b8a0c6 on The-Loeki:allow-telnet into b69c73b7cc9620567006c2e36212d65044abeb68 on napalm-automation:develop.

dbarrosop commented 7 years ago

Could you also document this optional_arg here, please?

https://github.com/napalm-automation/napalm/blob/master/docs/support/index.rst#list-of-supported-optional-arguments

Something like:

transport (ios, eos) - Transport to use when connecting to the device.  Supported transports are the ones supported by the underlying library. Defaults to https on EOS and ssh on IOS.

Thanks! :)

mirceaulinic commented 7 years ago

Postponed this for 0.8.0 as per @ktbyers' suggestion

matejv commented 7 years ago

is_alive method needs updating as well.

When using telnet remote_conn will be a telnetlib.Telnet instance that doesn't have a transport attribute. I'm not sure what is the right way to check if a telnet connection is active.

dbarrosop commented 7 years ago

Note there are CI issues we have to solve before merging.

ktbyers commented 7 years ago

I will update this. Hopefully this week or next week.

austind commented 7 years ago

@ktbyers Let me know if you need any help testing the PR

ktbyers commented 7 years ago

This change is incorporated here:

https://github.com/napalm-automation/napalm-ios/pull/192

austind commented 6 years ago

Successfully ran get_facts() against two telnet devices. Thank you!

ktbyers commented 6 years ago

@austind Nice!