Skip to content

Conversation

@dkaplan1
Copy link
Owner

@dkaplan1 dkaplan1 commented Jun 7, 2021

This PR addresses improvements that can be made to how nxapi_plumbing processes responses. The issue stems from different behavior in JSON-RPC vs XML responses, specifically return codes. JSON-RPC returns a 500 for an erroneous command, whereas XML returns a 200.

JSON

(Pdb) print(response)
<Response [500]>
(Pdb) print(response.json())
{'jsonrpc': '2.0', 'error': {'code': -32602, 'message': 'Invalid params', 'data': {'msg': 'SNMP community entry not found.\n'}}, 'id': 1}

XML

(Pdb) print(response)
<Response [200]>
(Pdb) print(response.text)
<?xml version="1.0"?>
<ins_api>
  <type>cli_conf</type>
  <version>1.0</version>
  <sid>eoc</sid>
  <outputs>
    <output>
      <clierror>SNMP community entry not found.
</clierror>
      <code>400</code>
      <msg>CLI execution error</msg>
    </output>
  </outputs>
</ins_api>

This means that erroneous commands using JSON-RPC will always raise NXAPIPostError with a generic error message including all commands with no indication of the underlying issue. https://github.com/napalm-automation/napalm/blob/d72e19444548bc921afd5a68bf46f2256ddee710/napalm/nxapi_plumbing/api_client.py#L81-L87

napalm.nxapi_plumbing.errors.NXAPIPostError: Invalid status code returned on NX-API POST
commands: [...]
status_code: 500

If we instead process JSON-RPC and XML differently, we can let XML continue to raise on non-200s and let JSON-RPC to return the result. Once the result is returned, _error_check will be called and can give better insight into what the problematic command was and information on the issue.

napalm.nxapi_plumbing.errors.NXAPICommandError: The command "..." gave the error "..."

In order to test this, I made some changes naming changes _send_request -> _send_request_to_device to make mocking easier and had to do a few things to plumb exposure of the status code.

Also, while going through the NXOS tests I noticed that we weren't raising when mocked_data included exception directives, so things like https://github.com/napalm-automation/napalm/blob/d72e19444548bc921afd5a68bf46f2256ddee710/test/nxos/mocked_data/test_get_interfaces_ip/no_ipv6_support/show_ipv6_interface.json#L1-L3 were not working as expected.

@dkaplan1 dkaplan1 changed the title Nxos improvements Update NXOS response processing for more informative error messaging Jun 7, 2021
@dkaplan1 dkaplan1 force-pushed the nxos-improvements branch from 11bd742 to 6334dbe Compare June 8, 2021 01:14
@dkaplan1 dkaplan1 force-pushed the nxos-improvements branch from 6334dbe to 6b29a50 Compare June 8, 2021 22:45
mirceaulinic and others added 21 commits June 14, 2021 13:16
…-bgp-flapcount

Fix bgp flap_count issue for iosxr_netconf
Bumps [pytest-cov](https://github.com/pytest-dev/pytest-cov) from 2.12.0 to 2.12.1.
- [Release notes](https://github.com/pytest-dev/pytest-cov/releases)
- [Changelog](https://github.com/pytest-dev/pytest-cov/blob/master/CHANGELOG.rst)
- [Commits](pytest-dev/pytest-cov@v2.12.0...v2.12.1)

---
updated-dependencies:
- dependency-name: pytest-cov
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [black](https://github.com/psf/black) from 21.5b1 to 21.6b0.
- [Release notes](https://github.com/psf/black/releases)
- [Changelog](https://github.com/psf/black/blob/main/CHANGES.md)
- [Commits](https://github.com/psf/black/commits)

---
updated-dependencies:
- dependency-name: black
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [mypy](https://github.com/python/mypy) from 0.812 to 0.902.
- [Release notes](https://github.com/python/mypy/releases)
- [Commits](python/mypy@v0.812...v0.902)

---
updated-dependencies:
- dependency-name: mypy
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
…s-eznc-2.6.1

Fix Junos test fixtures after junos-eznc 2.6.1 release
…ndabot/pip/pytest-cov-2.12.1

Bump pytest-cov from 2.12.0 to 2.12.1
…ndabot/pip/mypy-0.902

Bump mypy from 0.812 to 0.902
…ndabot/pip/black-21.6b0

Bump black from 21.5b1 to 21.6b0
Co-authored-by: Mircea Ulinic <mirceaulinic@users.noreply.github.com>
dependabot bot and others added 30 commits November 29, 2021 09:10
Bumps [coveralls](https://github.com/TheKevJames/coveralls-python) from 3.2.0 to 3.3.1.
- [Release notes](https://github.com/TheKevJames/coveralls-python/releases)
- [Changelog](https://github.com/TheKevJames/coveralls-python/blob/master/CHANGELOG.md)
- [Commits](TheKevJames/coveralls-python@3.2.0...3.3.1)

---
updated-dependencies:
- dependency-name: coveralls
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
…ndabot/pip/coveralls-3.3.1

Bump coveralls from 3.2.0 to 3.3.1
Added the ability to pass in optional arguments to the iosxr_netconf driver.  Note these are used as supplied and not passed through the netmiko_helpers base function.
Bumps [pytest-pythonpath](https://github.com/bigsassy/pytest-pythonpath) from 0.7.3 to 0.7.4.
- [Release notes](https://github.com/bigsassy/pytest-pythonpath/releases)
- [Commits](https://github.com/bigsassy/pytest-pythonpath/commits)

---
updated-dependencies:
- dependency-name: pytest-pythonpath
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…ndabot/pip/pytest-pythonpath-0.7.4

Bump pytest-pythonpath from 0.7.3 to 0.7.4
Bumps [mypy](https://github.com/python/mypy) from 0.910 to 0.931.
- [Release notes](https://github.com/python/mypy/releases)
- [Commits](python/mypy@v0.910...v0.931)

---
updated-dependencies:
- dependency-name: mypy
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
fix flaky test_unit.py::test__compare_getter_list
…ndabot/pip/mypy-0.931

Bump mypy from 0.910 to 0.931
Bumps [tox](https://github.com/tox-dev/tox) from 3.24.4 to 3.24.5.
- [Release notes](https://github.com/tox-dev/tox/releases)
- [Changelog](https://github.com/tox-dev/tox/blob/master/docs/changelog.rst)
- [Commits](tox-dev/tox@3.24.4...3.24.5)

---
updated-dependencies:
- dependency-name: tox
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…ndabot/pip/tox-3.24.5

Bump tox from 3.24.4 to 3.24.5
Bumps [tox](https://github.com/tox-dev/tox) from 3.23.1 to 3.24.3.
- [Release notes](https://github.com/tox-dev/tox/releases)
- [Changelog](https://github.com/tox-dev/tox/blob/master/docs/changelog.rst)
- [Commits](tox-dev/tox@3.23.1...3.24.3)

---
updated-dependencies:
- dependency-name: tox
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
…ndabot/pip/black-22.1.0

Bump black from 21.11b1 to 22.1.0
Add optional arguments support to IOSXR NETCONF driver
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 this pull request may close these issues.