fix: issue #110

[netgab]

Overview

Proposed code to fix issue#110

Added:

• Included connect method parameter rest_base_path with default value /restconf/data, which is used as the basic path for RESTCONF requests towards IOS-XE devices

Changed

• In the self.base_url for requests, the path rest_base_path is added. For example, the base_url changes from https://192.0.2.1:443 to https://192.0.2.1:443/restconf/data, which is the valid default path for IOS-XE devices.
• Within the connect method, the GET request to a common "well-known" IOS-XE YANG model (path: /Cisco-IOS-XE-native:native/version) is triggered to test access to the corresponding IOS-XE device.
• connect debug output to incude the received data from the device (IOS-XE version)
• connect now respects the Content-Type header set in the default_content_type parameter
• Test procedures for IOS-XE

Removed

• response in the connect method, because it does not bring and additional benefit to the method and to be consistent with other modules

[netgab]

Sorry (contrib newbie) ... I just saw, that the tests must be altered as well (src/rest/connector/tests/test_iosxe.py).
Currently, the def test_connect method is defined like no IOS-XE method reacts ... ... I'll check this as well.

[netgab]

I also modified the .gitignore, because I wanted to exclude a Python virtual environment folder. To run the test locally on my machine I did the following:

# Install needed packages into a virtual environment
python3 -m venv venv
python3 -m pip install --upgrade build
python3 -m pip install --upgrade pip
python3 -m pip install -e .
python3 -m pip install pyats[library]
python3 -m pip install requests_mock

# Run tests
python3 -m tests.test_iosxe

[netgab]

Also a side node and something to consider: Despite that the connect method does not work for IOS-XE devices at the moment, the module is currently designed, that users currently access the RESTCONF resources using the prefix /restconf/data<RESTCONF-RESOURCE>. Example is highlighted here.

My modifications change the user interface by defaulting to the /restconf/data base path as this is the default for IOS-XE. So a user formerly accessed a RESTCONF resource using:

device.rest.get("/restconf/data/IOS-XE-native:native")

with my modifications, the behavior changed to:

device.rest.get("/IOS-XE-native:native")

This breaks backwards compatibility to older scripts for IOS-XE devices.
However this is more consistent and user friendly in regards to RESTCONF. Just because RESTCONF resources in the YANG model are not described with the path /restconf/data.

For example, please refer to https://yangcatalog.org/yang-search/yang_tree/Cisco-IOS-XE-native@2022-07-01 The path to the corresponding resource is outlined using /ios:native or /Cisco-IOS-XE-native:native.
So a user does not need to care about the platform specific base path when using the module with my specifications. Just open the YANG model and use exactely the path (Sensor Path) as outlined.

However, to preserve backward compatibility, the code (and tests) may be modified, that only the connect method is fixed.

Anyway - for my specific problem I already implemented a local (for my use only) workaround and currently don't need RESTCONF in pyATS. If this PR is too much hassle for the code maintainers, please just close this PR without merging. I just wanted to start trying to contribute to this project. However, maybe this was not a good idea, because I have no idea about the contributing, development and design guidelines in your project.

[ThomasJRyan]

Hi there! Sorry it's taken so long to reply to this and thank you for the contribution!

Overall this does seem like a desirable change, however I see you've mentioned it would break backwards compatibility. As we have many people who use older versions and devices that is considered a large issue that we can't really allow to happen.

If you're able to rework this PR in a way that doesn't affect backwards compatibility then we would love to have the contribution!

[netgab]

Hi @ThomasJRyan, so I removed the default base URL for IOS-XE RESTCONF. The user still needs to pass the full URL for GET, POST, ... requests (so no behavior change).

Changelog:

Overview

Proposed code to fix issue#110

Changed

• Within the connect method, the GET request to a common "well-known" IOS-XE YANG model (path: /Cisco-IOS-XE-native:native/version) is triggered to test access to the corresponding IOS-XE device.
• connect debug output to include the received data from the device (IOS-XE version)
• connect now respects the Content-Type header set in the default_content_type parameter
• Test procedures for IOS-XE
• Docstring in Implementation(RestImplementation): The old docstring described, that the class is intended for wireless LAN controllers (eWLC). However, RESTCONF is supported by more newer IOS-XE models (e.g. Cat9k, ASR ...)

Removed

• response in the connect method, because it does not bring and additional benefit to the method and to be consistent with other modules

[netgab]

please add changelog

Where? Which file? Do you mean in docs/changelog/2023 ... Then I don't know which file, because I have no idea when you guys will merge my requests (will it be in Sept or Oct)? :) This is why I added the changelog here (above) that you guys may copy / paste it on the final release.

[omehrabi]

I see what you mean please under changelog create a new folder for undistributed and add the changlog over there

[netgab]

hey @ThomasJRyan and @omehrabi , I added the changelog as requested. Hopefully it's ok

[netgab]

Until next time :) Thanks for being so patient with me!