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

Cisco IOS merge and compare_config should generate an exception #224

Closed
ktbyers opened this issue Mar 8, 2016 · 8 comments
Closed

Cisco IOS merge and compare_config should generate an exception #224

ktbyers opened this issue Mar 8, 2016 · 8 comments

Comments

@ktbyers
Copy link
Contributor

ktbyers commented Mar 8, 2016

No description provided.

@ktbyers ktbyers self-assigned this Mar 8, 2016
@dbarrosop
Copy link
Member

Kirk, would it work to use the current running-config, append the changes at the end and use that file to compare?

Worst case, I think the best option would be to return the merge configuration instead of returning an Exception.

@ktbyers
Copy link
Contributor Author

ktbyers commented Mar 8, 2016

@dbarrosop No, just appending on to the end of running-config wouldn't work. A merge file could have 'no' commands. It could also have no to entire sections of config. For example, 'no router bgp 10'. You could also have merge config commands that modify a hierarchy 'router bgp 10 > neighbor 1.1.1.1 remote-as 20.

Yes, I think an exception is better than just returning the contents of the merge file. Returning the contents of the merge file gives the user a false sense of confidence (i.e. that it is actually doing a real comparison).

I guess we could prepend it with the message "Config merge not supported on Cisco IOS...returning merge config file contents" so that we made it clear in the output what it was doing. That way you wouldn't get an exception.

@dbarrosop
Copy link
Member

Yes, I think an exception is better than just returning the contents of the merge file. Returning the contents of the merge file gives the user a false sense of confidence (i.e. that it is actually doing a real comparison).

You are right. If we are going to throw an Exception I guess it's worth creating a new one, something like UnsupportedByDevice. I don't want to reuse NotImplementedException because that one means we haven't implemented it.

@ktbyers
Copy link
Contributor Author

ktbyers commented Apr 26, 2016

Merge operation should not generate this error:

(venv) ~/Documents/Projects/acl_manager$ python zz_napalm.py
Error: Could not open file flash0:/candidate_config.txt for reading

@ktbyers
Copy link
Contributor Author

ktbyers commented May 7, 2016

@dbarrosop Note, from seeing what other users are expecting here.

I changed my mind on this...I think I am going to implement .compare_config() and a merge operation to just echo the merge_config file (i.e. the changes the end-user submitted) with a comment stating that is what it is doing.

@ktbyers ktbyers removed their assignment May 7, 2016
@dbarrosop
Copy link
Member

Sounds good to me. Add it to the caveats : )

@ktbyers
Copy link
Contributor Author

ktbyers commented May 7, 2016

Okay, this is what this looks like on a compare_config() for merge operation.

! Cisco IOS does not support true compare_config() for merge: echo merge file.
+hostname pynet-rtr1z
+logging bufferred 9999

for a merge_file.txt that contains

hostname pynet-rtr1z
logging bufferred 9999

@dbarrosop
Copy link
Member

This issue has been 'moved' to napalm-automation/napalm-ios#26

dbarrosop pushed a commit that referenced this issue Oct 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants