oxidized
oxidized copied to clipboard
ytti
Ssh extraction
On May 7, 2017, at 5:54 AM, ytti <[email protected]mailto:[email protected]> wrote:
Thanks! You've put lot of work into this, but I'm not currently in position to test it. Perhaps @ElvinEfendihttps://github.com/ElvinEfendi or @danilopopeyehttps://github.com/danilopopeye
I've not read the extracted library code at all, but in the ssh.rb client using the library, at least #send is broken, and #cmd appears no longer to support expect but rely on execworking, which is true for minority of our models. I feel like there is still duplication and role-overwrap, like exec and disconnect
This was just short 5min review, sorry.
I have addressed each point except the 'exec' one. Any ideas?
#cmd is still broken, right? It doesn't not support expecting string as an end-of-output, but mandates use of exec channel?
Lot of devices I've tested do not support exec channels or support them incorrectly. IOS, IOS-XE and IOS-XR for example disconnect the whole SSH session after exec channel exits, requiring you to reconnect for every command. IOS-XR issue probably will be remedied in the 64b XRe, where SSH daemon should be changed to OpenSSH.
@ytti Check out the library! Specifically how the expectation_handler works! It is somewhat confusing to you because I am using your variable names with my code. Don't think of cmd as exec. They are not the same. cmd now calls SSHWrapper#exec with makes a decision on whether or not to request shell or exec channel
@danilopopeye @ElvinEfendi If I brought this into compliance with the current branch, would you mind checking it over? I know everybody is super busy, but this functionality would be amazing for users
@MajesticFalcon I'd be happy to review. What's the general overview of why this PR was submitted in the first place?
@laf Multiple reasons. SSH custom functionality shouldn't be a role of the main Oxidized. But for users likes me, I would love to use this SSH functionality in other scripts we use internally. It's ability to handle expect, tty, etc is currently difficult to come by.
I fully support that we actually should have
a) library to interact with networking deices b) configuration backup software using that library (now oxidized) c) scriptable CLI using that library (now oxidized-script)
the way oxidized script is now done is rather hacky. This separation is part of what I consider necessary in the issue where I've listed things rewrite would need.
@ytti so is this something you'd be happy to merge? Or is it something you'd want as an entire rewrite?
