napalm-automation / napalm-ios

Apache License 2.0
31 stars 40 forks source link

Send null byte when is_alive is polled #155

Closed mirceaulinic closed 7 years ago

mirceaulinic commented 7 years ago

As discussed with David B. under https://github.com/napalm-automation/napalm-base/pull/249, although it may be ambiguous that this method also executes an action, it's safer to determine the real status of the connection.

The paramiko flag is unreliable and it can return True even if the connection is actually unusable.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.004%) to 67.448% when pulling 5aee6a932a7251d2262ce43930edb19de05d61db on ios-alive into 094a60dd9b5f77a2c677fe1c86b61900ba786fca on develop.

mirceaulinic commented 7 years ago

@ktbyers do you think we can merge this PR then release a new version somewhere next week?

ktbyers commented 7 years ago

@mirceaulinic I will try to review this today and tomorrow. I am going to be out traveling a lot next week so I will try to get the PR integrated. Might be difficult for me to do a release. Happy for you to do one, however.

ktbyers commented 7 years ago

@mirceaulinic I asked this in the slack channel also? Why are we wrapping is_alive in a dictionary as opposed to just returning a boolean?

            return {
                'is_alive': False
            }
ktbyers commented 7 years ago

Never mind I see it in napalm-junos that way.