Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Library unmaintained and all kind of broken #51

Open
ytti opened this issue Apr 1, 2017 · 2 comments
Open

Library unmaintained and all kind of broken #51

ytti opened this issue Apr 1, 2017 · 2 comments

Comments

@ytti
Copy link

ytti commented Apr 1, 2017

  1. you need XML header, yet you specifically repeatedly call #root to get rid of it
  2. you treat hello as special case, instead of as part call utilising normal infrastructure, so it's missing MSG_END

Simple:

Netconf::SSH.new(login) do |nc|
  cfgall = nc.rpc.get_config
  puts cfgall
end

Won't work against latest IOS-XE (16.4.1), due to above errors.

On more larger infrastructure/design view:

  1. you should not call Nokogiri over once, you should wrap it on you own decorator so you can fix your code in single place
  2. you should not rely #send_and_receive to append the MSG_END this should be added by send function only, so commands which do not expect/want to receive data are still correctly appended by MSG_END
  3. all and all, it is seems POC/unmaintained/not_idiomatic, would be grand if Juniper would invest some time on money on it, I'm sure customer would enjoy it.

I do not propose fixing the 1/2, I propose redesign/refactor.

Thanks!

ytti added a commit to ytti/net-netconf that referenced this issue Apr 1, 2017
@ytti ytti mentioned this issue Apr 1, 2017
@dgjnpr
Copy link
Collaborator

dgjnpr commented Apr 5, 2017

thanks @ytti

I'm breathing new life into this project. Hopefully that will resolve point 3. For points 1 & 2 I agree that a refactor is required. But before that I want to setup CI so that we don't (seriously) break working existing behaviour

@tarko
Copy link

tarko commented Aug 29, 2017

+1, got hit with the malformatted HELLO on Timos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants