Skip to content
This repository was archived by the owner on Mar 13, 2025. It is now read-only.

Fix diff? method for newer JunOS versions#58

Merged
ydnath merged 1 commit intoJuniper:masterfrom
packethost:fix-diff
Apr 13, 2018
Merged

Fix diff? method for newer JunOS versions#58
ydnath merged 1 commit intoJuniper:masterfrom
packethost:fix-diff

Conversation

@lucasmpb
Copy link
Contributor

@lucasmpb lucasmpb commented Apr 5, 2018

There is an issue getting the diff? from a switch when there are no changes to be commited.
On JunOS ~14, ndev.rpc.get_configuration( :compare=>'rollback', :rollback=> rollback_id.to_s )returns an xml response:

<configuration-information>
<configuration-output>
</configuration-output>
</configuration-information>

But on JunOS ~17, the same method call, returns nil making the subsequent line to fail with undefined method 'xpath' for nil:NilClass

Adding :format => 'text' to the call seems to make the behavior consistent across different JunOS versions

PS: I took the idea from the more up to date python library https://github.com/Juniper/py-junos-eznc/blob/ce914d19030f51067b0c02c72fd69557f26c49e9/lib/jnpr/junos/utils/config.py#L225

def diff?( rollback_id = 0 )
raise ArgumentError, "invalid rollback #{rollback_id}" unless ( rollback_id >= 0 and rollback_id <= 50 )
got = ndev.rpc.get_configuration( :compare=>'rollback', :rollback=> rollback_id.to_s )
got = ndev.rpc.get_configuration( :compare => 'rollback', :rollback => rollback_id.to_s, :format => 'text' )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucasmpb why do we need to add format? What changed in the newer version of Junos?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @vnitinv I should have explained myself. I added a description to the pull request.

@vnitinv
Copy link
Contributor

vnitinv commented Apr 13, 2018

@lucasmpb I agree here it should be format text only. But scared if this change will break any existing user code. Will discuss with team.

@ydnath Can you review this. I think this change should go in.

@ydnath
Copy link
Member

ydnath commented Apr 13, 2018

Checked and does not impact north bound api

@ydnath ydnath merged commit 5bbab24 into Juniper:master Apr 13, 2018
@ydnath ydnath mentioned this pull request Apr 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants