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

NX-OS refactor #833

Merged
merged 24 commits into from Oct 25, 2018
Merged

NX-OS refactor #833

merged 24 commits into from Oct 25, 2018

Conversation

ktbyers
Copy link
Contributor

@ktbyers ktbyers commented Oct 13, 2018

I want to release napalm 2.3.3 before merging this.

This is a big change to both the NX-OS drivers

@ktbyers
Copy link
Contributor Author

ktbyers commented Oct 13, 2018

Ran napalm_custom_test on nxos and nxos_ssh for this updated driver. I also did quite a bit of testing using pdb and a real device.

https://github.com/ktbyers/napalm_custom_test

@coveralls
Copy link

coveralls commented Oct 13, 2018

Coverage Status

Coverage increased (+1.3%) to 80.394% when pulling a62319e on ktbyers:nxos_refactor_scp into 6de9af6 on napalm-automation:develop.

@ktbyers ktbyers mentioned this pull request Oct 13, 2018
@ktbyers
Copy link
Contributor Author

ktbyers commented Oct 13, 2018

Added unit tests to test that automatic rollback on a failed merge operation worked properly for NX-OS (this is in the testing against real devices). Currently, I only ran it as a one off test; not running on an ongoing basis (it has to be merged into 'develop' before I can test on an ongoing basis).

ktbyers/napalm_custom_test@37a961f#diff-16b358213f85e70673686e009a79b591R93

@ktbyers ktbyers changed the title [Do Not Merge] NX-OS refactor NX-OS refactor Oct 15, 2018
@mattschwen
Copy link

+1

I have reviewed the code briefly for a sanity check and have tested the PR changes against physical hardware. I used multiple getters for NXOS and NXOS_SSH as well as config operations on both 9K and 5k with no tracebacks or odd behavior.

Copy link
Member

@bewing bewing left a comment

Choose a reason for hiding this comment

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

LGTM.

@ktbyers ktbyers merged commit d640449 into napalm-automation:develop Oct 25, 2018
ExaneServerTeam pushed a commit to ExaneServerTeam/napalm that referenced this pull request Mar 6, 2020
* Refactoring to use Netmiko SCP

* Restructuring code to use Netmiko SCP

* Consolidating common code.

* Integrating nxos_ssh and nxos together better

* Working on testing restructured code

* Fixing decorator reference

* Adding platform attribute; fixing decorator bug

* Working on updating NXOS driver

* Linting cleanup

* Fixing bug with diff generation

* More nxos consolidationg work

* Linting cleanup

* Removing some comments

* Fix decorator and signature mismatch on test

* Make nxos merge more atomic

* Fix issue with not returning config change status

* Fixing issue with sot_file overwrite

* Fixing spelling

* Fixing bug with nxos_ssh rollback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants