networktocode / jdiff Goto Github PK
View Code? Open in Web Editor NEWHome Page: https://jdiff.readthedocs.io/en/latest/
License: Apache License 2.0
Home Page: https://jdiff.readthedocs.io/en/latest/
License: Apache License 2.0
Based on discussion: https://github.com/networktocode-llc/netcompare/pull/63#discussion_r895696463
As it is now, operator
check has grater than - gt
and lower than - lt
types. Would nice also to have greater or equal than - gte
as well as lower or equal than - lte
In case an user wants to get extract just one element from the json object, he has to pass it in a list anyway
"global.$peers$.*.is_enabled"
won't work
returns: ({'index_element[3]': {'new_value': False, 'old_value': True}}, False)
"global.$peers$.*.[is_enabled]"
will work
returns: ({'7.7.7.7': {'is_enabled': {'new_value': False, 'old_value': True}}}, False)
We can either fix or document as requirement
Based on @dgarros suggestion, would be great to implement range
as check_type
.
As reference: https://github.com/Juniper/jsnapy/wiki#supported-test-operators-and-their-description
Based on this discussion, I believe would be great to have jmspath
arg mandatory. That would keep code consistent and make less confused the user regarding the functionality of such arg. In case we would need raw
diff, *
is possible to use it
Some part of the code, does not have typing
implemented. For consistency would be good to implement in all of them.
As it stands now, jdiff
can catch a reference key only in a list of dictionary. There are case where NAPALM returns dict of dicts. In the example below, we want .local.
and .local..0
to be the ref key of is_up
{
".local.": {
"description": "",
"is_enabled": true,
"is_up": true,
"last_flapped": -1,
"mac_address": "Unspecified",
"mtu": 0,
"speed": -1
},
".local..0": {
"description": "",
"is_enabled": true,
"is_up": true,
"last_flapped": -1,
"mac_address": "Unspecified",
"mtu": 0,
"speed": -1
},
}
@pszulczewski FYI
fix_deepdiff_key_names()
function causes deepdiff
results to be overwritten with the latest one.
When comparing data structured with textfsm templates (data structure from textfsm is usually a list of dictionaries) the following bug has been identified.
When fix_deepdiff_key_names()
function aggregates keys from deepdiff
output it doesn't recognize different elements and overwrites these elements returning only the last identified difference.
Compare mock files to understand differences in tests/mock/textfsm_ospf_int_br
Checkout issue branch textfsm_overwritten
and run test: pytest -k textfsm_ospf_int_br
diff_generator
returns:
({'state': {'new_value': 'DR', 'old_value': 'BDR'}}, False)
when DeepDiff
correctly identifies differences as:
{'values_changed': {"root[1]['state']": {'new_value': 'DOWN', 'old_value': 'P2P'}, "root[2]['state']": {'new_value': 'DR', 'old_value': 'BDR'}}}
@pszulczewski https://github.com/networktocode-llc/netcompare/pull/34 introduced a bug:
>>> from netcompare import CheckType
>>> pre_data = {
"jsonrpc": "2.0",
"id": "EapiExplorer-1",
"result": [
{
"interfaces": {
"Management1": {
"lastStatusChangeTimestamp": 1626247820.0720868,
"lanes": 0,
"name": "Management1",
"interfaceStatus": "connected",
"autoNegotiate": "success",
"burnedInAddress": "08:00:27:e6:b2:f8",
"loopbackMode": "loopbackNone",
"interfaceStatistics": {
"inBitsRate": 3582.5323982177174,
"inPktsRate": 3.972702352461616,
"outBitsRate": 17327.65267220522,
"updateInterval": 300,
"outPktsRate": 2.216220664406746
}
}
}
}
]
}
>>> post_data = {
"jsonrpc": "2.0",
"id": "EapiExplorer-1",
"result": [
{
"interfaces": {
"Management1": {
"lastStatusChangeTimestamp": 1626247821.123456,
"lanes": 0,
"name": "Management1",
"interfaceStatus": "down",
"autoNegotiate": "success",
"burnedInAddress": "08:00:27:e6:b2:f8",
"loopbackMode": "loopbackNone",
"interfaceStatistics": {
"inBitsRate": 3403.4362520883615,
"inPktsRate": 3.7424095978179257,
"outBitsRate": 16249.69114419833,
"updateInterval": 300,
"outPktsRate": 2.1111866059750692
}
}
}
}
]
}
>>> my_jmspath = "result[*].interfaces.$Management1$.interfaceStatus"
>>> post_value = my_check.get_value(output=post_data, path=my_jmspath)
>>> pre_value = my_check.get_value(output=pre_data, path=my_jmspath)
>>> pre_value
['connected']
>>> post_value
['down']
>>> result = my_check.evaluate(value_to_compare=post_value, reference_data=pre_value)
>>> result
({'index_element[0]': {'new_value': 'down', 'old_value': 'connected'}}, False)
In that case, we use $Management1$
as reference key so I would expect result to be:
({'Management1': {'new_value': 'down', 'old_value': 'connected'}}, False)
Can we have a look together unless you already know how to fix it?
@chadell would be great if you can draft an idea of how you would refactor evaluator.py
Taking the following JMSPath expression result[0].vrfs.default.peerList[*].[$peerAddress$,localAsn,linkType]
we should have a parsed output which includes only localAsn
and linkType
.
We also get instead the reference key
[{'7.7.7.7': {'peerAddress': '7.7.7.7', 'localAsn': '65130.1010', 'linkType': 'the road to seniority'}}, {'10.1.0.0': {'peerAddress': '10.1.0.0', 'localAsn': '65130.8888', 'linkType': 'external'}}, {'10.2.0.0': {'peerAddress': '10.2.0.0', 'localAsn': '65130.1100', 'linkType': 'external'}}, {'10.64.207.255': {'peerAddress': '10.64.207.255', 'localAsn': '65130.1100', 'linkType': 'external'}}]
As suggested by OSRB code review, we have implemented mypy
in tests. We need to fix all the errors returned
As we have some assertion within the CheckType
Class, we would need to add tests in order to evaluate that such assertion are raising the error as expected. For example: regex implementation would need to test all the below
try:
parameter = value_to_compare[1]
except IndexError as error:
raise IndexError(
f"Evaluating parameter must be defined as dict at index 1. You have: {value_to_compare}"
) from error
# Assert that check parameters are at index 1.
if not all([isinstance(parameter, dict)]):
raise TypeError("check_option must be of type dict().")
# Assert that check option has 'regex' and 'mode' dict keys.
if not "regex" in parameter and "mode" in parameter:
raise KeyError(
"Regex check-type requires check-option. Example: dict(regex='.*UNDERLAY.*', mode='no-match')."
)
# Assert that check option has 'regex' and 'mode' dict keys.
if parameter["mode"] not in ["match", "no-match"]:
raise ValueError(
"Regex check-type requires check-option. Example: dict(regex='.*UNDERLAY.*', mode='no-match')."
)
As per json below, we might have multiple vrf and for each vrf multiple peer IPs - which are unique within each vrf. In the example below, we want to anchor global
and 192.168.0.0
. So regex would look like: $*$.peers.$*$.*.*.[accepted_prefixes,received_prefixes,sent_prefixes]
{
"global": {
"router_id": "10.255.255.1",
"peers": {
"192.168.0.0": {
"description": "",
"remote_id": "10.255.255.240",
"address_family": {
"ipv6": {
"received_prefixes": -1,
"sent_prefixes": -1,
"accepted_prefixes": -1
},
"ipv4": {
"received_prefixes": -1,
"sent_prefixes": -1,
"accepted_prefixes": -1
}
},
"remote_as": 30000,
"is_up": true,
"local_as": 30001,
"is_enabled": true,
"uptime": 699262
>>> post_data = {
... "jsonrpc": "2.0",
... "id": "EapiExplorer-1",
... "result": [
... {
... "interfaces": {
... "Management1": {
... "lastStatusChangeTimestamp": 1626247821.123456,
... "lanes": 0,
... "name": "Management1",
... "interfaceStatus": "down",
... "autoNegotiate": "success",
... "burnedInAddress": "08:00:27:e6:b2:f8",
... "loopbackMode": "loopbackNone",
... "interfaceStatistics": {
... "inBitsRate": 3403.4362520883615,
... "inPktsRate": 3.7424095978179257,
... "outBitsRate": 16249.69114419833,
... "updateInterval": 300,
... "outPktsRate": 2.1111866059750692
... }
... }
... }
... }
... ]
... }
>>>
>>> my_check = CheckType.init(check_type="parameter_match")
>>> my_jmspath = "result[*].interfaces.*.[$name$,interfaceStatus,autoNegotiate]"
>>> post_value = my_check.get_value(output=pre_data, path=my_jmspath)
>>> my_parameter_match = {"mode": "match", "params": {"interfaceStatus": "connected", "autoNegotiate": "success"}}
>>> actual_results = my_check.evaluate(post_value, **my_parameter_match)
>>> actual_result
({}, True)
>>> post_value
[{'Management1': {'interfaceStatus': 'down', 'autoNegotiate': 'success'}}]
>>> my_parameter_match = {"mode": "no-match", "params": {"interfaceStatus": "connected", "autoNegotiate": "success"}}
>>> actual_results = my_check.evaluate(post_value, **my_parameter_match)
>>> actual_result
({}, True)
(<dict whose structure depends on the check type>, ,<boolean>)
the ideal API for this library? Or would it make sense to define some custom classes/data structures to return, in order make things more user-friendly and more flexible for future expansion?pylint
and pytest
in containers, but the other linting can run with INVOKE_LOCAL=True
?E5110
might be or why it's disabled.tests/unit/
and tests/integration
directories are just cookiecutter boilerplate - consider deleting them.flatten_list
flattens [[[[-1, 0], [-1, 0]]]]
to [[-1, 0], [-1, 0]]
rather than say [-1, 0, -1, 0]
- consider adding more details to its docstring to make it clearer what it considers to be a "flattened" list and what it doesn't.exclude_filter
as implemented accepts either a dict or a list for the data
parameter, but the type annotations say it takes a Mapping
- perhaps this should be Union[Dict, List]
instead?test_diff_generator
and test_operators
especially, is there a reason that the pre/post data are stored as separate JSON files, while the other test data are defined inline in the test scripts? Might be more consistent and more maintainable to either store it all as JSON or all as code.test_sw_upgrade_device_state
- would renaming test_is_passed
to check_should_pass
be more accurate?netcompare.utils.diff_helpers
? I see TODOs for a couple of them but I may have overlooked testing for the ones that don't have TODOs either.mypy
to the linters in order to validate your type hinting is correct. Not blocking.netcompare.utils.jmespath_parsers
would benefit from more comprehensive docstrings, IMO.CheckType.get_value()
only supports/uses exclude
when output
is a Dict? Given that exclude_filter
explicitly can handle both dicts and lists? If this is by design, consider raising an exception in the "else" case rather than silently ignoring the exclude
parameter.ToleranceType
will reject tolerance=0
as invalid - intentional?tolerance
must be an int, rather than say a float?ToleranceType
check that tolerance
is between 0 and 100? Or are there valid use cases for a >100% tolerance?validate
signature, e.g., for ToleranceType this would be def validate(*, tolerance: int)
. Also, is there a reason for the concrete implementations of validate
to accept wildcard **kwargs
at all?Operator
, referance_data
should be reference_data
throughout.Operator
, more detailed docstrings would be appreciated.As suggested by @davidban77 would be great to implement check_type: regex
As I see this, should be a python regex that can match a value of type str()
When providing JMESPath expression to get_value
mathod that doesn't produce result I expect to receive empty dict or None
.
An exception is raised, see traceback below:
(.venv) przemek@quark:~/osrb/tests/netcompare$ /home/przemek/osrb/tests/netcompare/.venv/bin/python /home/przemek/osrb/tests/netcompare/bug.py
Traceback (most recent call last):
File "/home/przemek/osrb/tests/netcompare/bug.py", line 7, in <module>
pre_value = my_check.get_value(output=data, path=my_jmspath)
File "/home/przemek/osrb/tests/netcompare/.venv/lib/python3.8/site-packages/netcompare/check_types.py", line 77, in get_value
if not any(isinstance(i, list) for i in values):
TypeError: 'NoneType' object is not iterable
Use the below code to reproduce this scenario:
from netcompare import CheckType
data = {"global": {"peers": {"10.1.0.0": "peer1", "10.2.0.0": "peer2"}}}
my_jmspath = "global[*]"
my_check = CheckType.init(check_type="exact_match")
pre_value = my_check.get_value(output=data, path=my_jmspath)
Based on the discussion on #85, would be great to give more granular control to the operator check give an argument such as all_items
where we want check if all the items in data are matching the operator logic.
I believe we can work with one file only for mock data. Would be great if we can cleanup all the redundant files as well as give some naming convention
@chadell As part of code refactoring, I am going through all the tests included for the library. I believe there is partial overlapping between test_type_check.py
and test_diff_generator.py
. Whenever you have chance, could you please have a look?
@chadell would be great if you can draft an idea of how you would refactor runner.py
So we can reserve the name on PyPi repo
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.