Adding traceroute method for Cisco IOS

napalm-automation / napalm-ios

Apache License 2.0

31 stars 40 forks source link

Adding traceroute method for Cisco IOS #72

Closed ubajze closed 7 years ago

ubajze commented 7 years ago

Adding traceroute method.

I was testing on attached output and it is working fine. Not sure if I hit all corner cases, I was trying to make the output as complex as possible.

Uros

mirceaulinic commented 7 years ago

@ubajze The test is failing, as the data returned does not match the models:

-------------------- >> begin captured stdout << ---------------------
key: rtt
model_class: <type 'float'>
data_class: <type 'NoneType'>
key: ip_address
model_class: <type 'unicode'>
data_class: <type 'NoneType'>
key: host_name
model_class: <type 'unicode'>
data_class: <type 'NoneType'>

You should return 0.0 for rtt and empty string for ip_address and host_name as defaults, in case of unresponsive hop.

ubajze commented 7 years ago

Just found out about returned data, I was returning None when hop was not responsive... Fixed that in latest commit

mirceaulinic commented 7 years ago

You're getting close:

key: ip_address
model_class: <type 'unicode'>
data_class: <type 'str'>
key: host_name
model_class: <type 'unicode'>
data_class: <type 'str'>

Please make sure you return unicode. To make it Py2 and Py3 compatible, cast the datatype using: py23_compat.text_type(ip_address) everywhere - the same for host_name.

coveralls commented 7 years ago

Coverage increased (+1.08%) to 69.988% when pulling 20919953ce37460a67e14a53f7cce1d8aeec556e on ubajze:develop into ed75dbe4293237ddeb7d55ed0d0a21277b354bfc on napalm-automation:develop.

coveralls commented 7 years ago

Coverage increased (+1.2%) to 70.058% when pulling 9976ea2978e57cb7c9e289a0489c8996221d151f on ubajze:develop into ed75dbe4293237ddeb7d55ed0d0a21277b354bfc on napalm-automation:develop.

coveralls commented 7 years ago

Coverage increased (+1.2%) to 70.058% when pulling 9976ea2978e57cb7c9e289a0489c8996221d151f on ubajze:develop into ed75dbe4293237ddeb7d55ed0d0a21277b354bfc on napalm-automation:develop.

ubajze commented 7 years ago

Finally all checks have passed :)

ktbyers commented 7 years ago

@ubajze I will try to review this shortly.

ktbyers commented 7 years ago

I will fix this to work with the new testing framework.

ktbyers commented 7 years ago

I merged this into my devel branch and fixed the tests to use the new testing framework. I also made a few minor changes.

Closing this PR and will link to new PR.

ktbyers commented 7 years ago

This PR has been incorporated here:

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