-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
Fix iosxr netconf plugin response namespace #49238
Conversation
* iosxr netconf plugin removes namespace by default for all the responses as parsing of xml is easier without namepsace in iosxr module. However to validate the response received from device against yang model requires namespace to be present in resposne. * Add a parameter in iosxr netconf plugin to control if namespace should be removed from response or not.
Hi @ganeshrn, thank you for submitting this pull-request! |
The test
|
@@ -389,7 +392,10 @@ def get_oper(module, filter=None): | |||
|
|||
if filter is not None: | |||
try: | |||
response = conn.get(filter) | |||
if is_netconf(module): | |||
response = conn.get(filter, remove_ns=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is existing bug in this code. iosxr netconf connection plugin's 'get' method does not have any non-keyword argument. so here probably passing a keyword arg might not work.
'conn.get(filter=filter, remove_ns=True)'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is fixed on devel, however, I will change as you suggested since same iosxr plugin will be copied in yang role that is currently dependent on 2.7 Ansible version.
@@ -436,7 +442,7 @@ def load_config(module, command_filter, commit=False, replace=False, | |||
|
|||
try: | |||
for filter in to_list(command_filter): | |||
conn.edit_config(filter) | |||
conn.edit_config(filter, remove_ns=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. below should work
- conn.edit_config(filter)
+ conn.edit_config(config=filter, remove_ns=True)
@@ -125,56 +125,80 @@ def guess_network_os(obj): | |||
|
|||
# TODO: change .xml to .data_xml, when ncclient supports data_xml on all platforms | |||
@ensure_connected | |||
def get(self, filter=None): | |||
def get(self, filter=None, remove_ns=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure of history but why do we need remove_ns at first place ? is it for conversion to json ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc this logic was added to iosxr plugin so that parsing of XML response string using lxml library in the iosxr declarative modules is easier. If the namespace is present in the XML string the XPath traversal in declarative modules requires adding nested namespaces to traversal logic thus making it complex.
@gdpak Thank you for review. |
* Fix iosxr netconf plugin response namespace * iosxr netconf plugin removes namespace by default for all the responses as parsing of xml is easier without namepsace in iosxr module. However to validate the response received from device against yang model requires namespace to be present in resposne. * Add a parameter in iosxr netconf plugin to control if namespace should be removed from response or not. * Fix CI issues * Fix review comment (cherry picked from commit 829fc0f)
* Fix iosxr netconf plugin response namespace * iosxr netconf plugin removes namespace by default for all the responses as parsing of xml is easier without namepsace in iosxr module. However to validate the response received from device against yang model requires namespace to be present in resposne. * Add a parameter in iosxr netconf plugin to control if namespace should be removed from response or not. * Fix CI issues * Fix review comment (cherry picked from commit 829fc0f)
SUMMARY
for all the responses as parsing of xml is easier without namespace in iosxr module. However, to
validate the response received from the device against yang model requires namespace to be
present.
should be removed from the response or not.
ISSUE TYPE
COMPONENT NAME
netconf/iosxr.py
ADDITIONAL INFORMATION