Open moophat opened 4 years ago
Upon further inspection, look like the check operators also can not work if multi id was specified and one of the specified id has None value. On the other hand they do not have any problem if working on multi-id without any None value involved.
The most simple example is commands that involve S-VLAN and C-VLAN combination, specify both S-vlan and C-VLAN will make the test fail if either of them is none on any node. The logic of the code certainly have its merit, but from a network operation perspective it's incorrect because a combination of (S:20, C:21) is just as valid as (S:None, C:21), both of them represent an unique vlan-tagging scenario.
We just wonder there anyone available to discuss possible optimization?
Description of Issue/Question
Any Jsnapy comparison operator like no-diff or delta will simply do not execute if the xml xpath was not specified together with an "id" field.
This seem logical at first, since with a set of data (A1, B1, C1) (3 nodes extracted by xpath from xml output) on pre_snapshot_file, and another set of data (A2, B2, C2, D2) on post_snapshot_file, we need something that let the code know that it suppose to compare A1 against A2 (not B2, C2 or D2)
However in some case, there's nothing in the xml element that can act as an identifier, but comparison still need to be and supposedly can be done (given that user use the right xpath), like the example below:
Setup
Any xml that has its elements identify by a junos attribute, instead of a sub-element (or simply do not have any identifier) would led to this situation. This is easily seen on many BNG router output.
This is a very simple test, being made difficult by the way Junos handle the styling of the xml output
The above delta simply do not work, Jsnapy will report that node list was empty, because we did not follow an ID after xpath.
Steps to Reproduce Issue
Looking at the code, it's easily to found out that if id was not specified, an empty id_list was generated https://github.com/Juniper/jsnapy/blob/de14e22fabebce80b85bb14bc41bebbb1a2c034b/lib/jnpr/jsnapy/check.py#L413 This empty list will then be passed into the Operator object https://github.com/Juniper/jsnapy/blob/de14e22fabebce80b85bb14bc41bebbb1a2c034b/lib/jnpr/jsnapy/check.py#L433 In the Operator's method, we try to get two list of node(s) using this id https://github.com/Juniper/jsnapy/blob/a0c7fef0a209c5a9f0f54c78e3c2e095a5b8ad5e/lib/jnpr/jsnapy/operator.py#L2440 But the Operator's data retriever do nothing if the id_list is empty https://github.com/Juniper/jsnapy/blob/a0c7fef0a209c5a9f0f54c78e3c2e095a5b8ad5e/lib/jnpr/jsnapy/operator.py#L211
The way Jsnapy current working, any simple statistic command like these will also have problem because there's no xml sub-element to be used as "id"
Versions Report
Version doesn't seem to matter because this is inherently a matter with the way Junos handle some output and such case was not covered by jsnapy.
Suggestion
I understand the need to have a union of pre/post nodes (represented in the code by keys_union) - upon which we can perform comparison on matching element using their id. But the way Junos handle these type of format and the way Jsnapy rely wholly on item id is making some very simple comparison impossible.
A workaround would be to check if the pre_nodes and post_nodes have only 1 element (since pre_nodes and post_nodes are extracted xpath, not id). Then allow the comparison to proceed without using any keys_union things in such case.
Of course for this method to work the user must make sure that their xpath result in one single node . But it is almost always the case for the types of BNG output I laid out above and will be useful for many other case where xpath or the xml output itself do not result in multi-node and there's no xml element to be used as ID.