Fix parameter_match check, to support non-normalized data #90

pszulczewski · 2022-10-26T14:41:10Z

While working on some Napalm checks, another time I faced issues that some of checks are expecting only normalized data with jmespath anchored key. As this is limiting usability of the lib, hence the fix for parameter_match.
It still supports normalized data, but now also supports flat dictionaries.

importlib-metadata was failing for flake8 checks. New release 5.0.0 is not backwards compatible so keeping it below 5.0.0 for the time being.

pszulczewski · 2022-10-26T14:43:32Z

tests/test_sw_upgrade_device_state.py

-        ("arista_eos", "show_version", "[*].[$image$,image]", {"image": "4.14.7M"}, True),
-        ("arista_eos", "show_version", "[*].[$image$,image]", {"image": "no-match"}, False),
-        ("cisco_ios", "show_version", "[*].[$version$,version]", {"version": "12.2(54)SG1"}, True),
-        ("cisco_ios", "show_version", "[*].[$version$,version]", {"version": "no-match"}, False),
-        ("cisco_nxos", "show_version", "[*].[$os$,os]", {"os": "6.1(2)I3(1)"}, True),
-        ("cisco_nxos", "show_version", "[*].[$os$,os]", {"os": "no-match"}, False),


That was very unnatural workaround for parameter_match. Now fixed.

pszulczewski · 2022-10-26T14:45:28Z

jdiff/evaluators.py

@@ -51,41 +51,41 @@ def parameter_evaluator(values: Mapping, parameters: Mapping, mode: str) -> Dict
        Dictionary with all the items that have some value not matching the expectations from parameters
    """
    if not isinstance(values, list):
-        raise TypeError("Something went wrong during jmespath parsing. 'values' must be of type List.")


Jmespath parsing is now optional.

Question around this. If JMSPATH is now optional, how the library will understand when is required and when is not?

chadell · 2022-10-27T05:45:31Z

jdiff/evaluators.py

+            # inner_value: [{'peerAddress': '7.7.7.7', 'localAsn': '65130.1101', 'linkType': 'externals'}]
+            value = list(value.values())[0]
+        else:
+            inner_key = index


what is the value in this case?

An element from the top for loop, where values is a function argument.

If data normalisation is detected, then reassign value from inner element.

Jmespath parsing is now optional.

I believe this answering to my question above (?)

lvrfrc87 · 2022-11-10T16:39:10Z

@pszulczewski Please update tests for parameter_match with the use case. Then would be good to go.

pszulczewski · 2022-11-10T17:15:59Z

@pszulczewski Please update tests for parameter_match with the use case. Then would be good to go.

@lvrfrc87 test added requested.

lvrfrc87

Just one point from my side. If you can clarify we should then be good to go

lvrfrc87 · 2022-11-11T07:51:01Z

jdiff/evaluators.py

@@ -51,41 +51,41 @@ def parameter_evaluator(values: Mapping, parameters: Mapping, mode: str) -> Dict
        Dictionary with all the items that have some value not matching the expectations from parameters
    """
    if not isinstance(values, list):
-        raise TypeError("Something went wrong during jmespath parsing. 'values' must be of type List.")


Question around this. If JMSPATH is now optional, how the library will understand when is required and when is not?

lvrfrc87 · 2022-11-11T07:52:28Z

jdiff/evaluators.py

+            # inner_value: [{'peerAddress': '7.7.7.7', 'localAsn': '65130.1101', 'linkType': 'externals'}]
+            value = list(value.values())[0]
+        else:
+            inner_key = index


Jmespath parsing is now optional.

I believe this answering to my question above (?)

pszulczewski · 2022-11-15T00:12:32Z

@lvrfrc87 there is detection is data has been normalized.
jmespath is indeed optional, you may have flat data like napalm facts, where you want to do parameter match (use case).

lvrfrc87 · 2022-11-15T09:26:50Z

LGTM!

* Update readme to start with use cases (#84) * Update readme to start with use cases * Apply suggestions from code review Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Update README.md Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Doc update (#87) * Fix operator checks to follow other check_type logic. (#85) * Fix operator checks to follow other check_type logic. * Add new release 0.0.2 Co-authored-by: Patryk Szulczewski <[email protected]> * Implement `ge` and `le` operator type (#89) * Release 0.0.2 (#88) * Update readme to start with use cases (#84) * Update readme to start with use cases * Apply suggestions from code review Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Update README.md Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Doc update (#87) * Fix operator checks to follow other check_type logic. (#85) * Fix operator checks to follow other check_type logic. * Add new release 0.0.2 Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Christian Adell <[email protected]> Co-authored-by: Ken Celenza <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> * add operator ge and le * fix validate tests Co-authored-by: Christian Adell <[email protected]> Co-authored-by: Ken Celenza <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> * Fix parameter_match check, to support non-normalized data (#90) * Fix parameter_match check, to support non-normalized data * Add test * Update tests. * remove test print Co-authored-by: Patryk Szulczewski <[email protected]> * Update mypy * Bugfix to data normalization * Fix ref_key parsing * Update docs * Issue 92 (#98) * Base-line tests for future refactor. * Refactor to simplify code. * Implement issue #92 * Bump certifi from 2022.9.24 to 2022.12.7 (#95) Bumps [certifi](https://github.com/certifi/python-certifi) from 2022.9.24 to 2022.12.7. - [Release notes](https://github.com/certifi/python-certifi/releases) - [Commits](certifi/python-certifi@2022.09.24...2022.12.07) --- updated-dependencies: - dependency-name: certifi dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Christian Adell <[email protected]> Co-authored-by: Ken Celenza <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update readme to start with use cases (#84) * Update readme to start with use cases * Apply suggestions from code review Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Update README.md Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Doc update (#87) * Fix operator checks to follow other check_type logic. (#85) * Fix operator checks to follow other check_type logic. * Add new release 0.0.2 Co-authored-by: Patryk Szulczewski <[email protected]> * Implement `ge` and `le` operator type (#89) * Release 0.0.2 (#88) * Update readme to start with use cases (#84) * Update readme to start with use cases * Apply suggestions from code review Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Update README.md Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Doc update (#87) * Fix operator checks to follow other check_type logic. (#85) * Fix operator checks to follow other check_type logic. * Add new release 0.0.2 Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Christian Adell <[email protected]> Co-authored-by: Ken Celenza <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> * add operator ge and le * fix validate tests Co-authored-by: Christian Adell <[email protected]> Co-authored-by: Ken Celenza <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> * Fix parameter_match check, to support non-normalized data (#90) * Fix parameter_match check, to support non-normalized data * Add test * Update tests. * remove test print Co-authored-by: Patryk Szulczewski <[email protected]> * Update mypy * Bugfix to data normalization * Fix ref_key parsing * Update docs * Issue 92 (#98) * Base-line tests for future refactor. * Refactor to simplify code. * Implement issue #92 * Bump certifi from 2022.9.24 to 2022.12.7 (#95) Bumps [certifi](https://github.com/certifi/python-certifi) from 2022.9.24 to 2022.12.7. - [Release notes](https://github.com/certifi/python-certifi/releases) - [Commits](certifi/python-certifi@2022.09.24...2022.12.07) --- updated-dependencies: - dependency-name: certifi dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update version and changelog * update poetry lock * Update changelog * Issue 94 & 91 (#100) * Baseline unittests * Fix issue #94 * Remove test print * Fix issue #91 * Relax deepdiff dependency. * Update pyproject.toml Co-authored-by: Patryk Szulczewski <[email protected]> * Update poetry.lock * Relase v0.0.4 --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Christian Adell <[email protected]> Co-authored-by: Ken Celenza <[email protected]> Co-authored-by: Stephen Corry <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Federico87 <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Network to Code <[email protected]> Co-authored-by: Leo Kirchner <[email protected]> Co-authored-by: Leo Kirchner <[email protected]>

* Update readme to start with use cases (#84) * Update readme to start with use cases * Apply suggestions from code review Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Update README.md Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Doc update (#87) * Fix operator checks to follow other check_type logic. (#85) * Fix operator checks to follow other check_type logic. * Add new release 0.0.2 Co-authored-by: Patryk Szulczewski <[email protected]> * Implement `ge` and `le` operator type (#89) * Release 0.0.2 (#88) * Update readme to start with use cases (#84) * Update readme to start with use cases * Apply suggestions from code review Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Update README.md Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> * Doc update (#87) * Fix operator checks to follow other check_type logic. (#85) * Fix operator checks to follow other check_type logic. * Add new release 0.0.2 Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Christian Adell <[email protected]> Co-authored-by: Ken Celenza <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> * add operator ge and le * fix validate tests Co-authored-by: Christian Adell <[email protected]> Co-authored-by: Ken Celenza <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Stephen Corry <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> * Fix parameter_match check, to support non-normalized data (#90) * Fix parameter_match check, to support non-normalized data * Add test * Update tests. * remove test print Co-authored-by: Patryk Szulczewski <[email protected]> * Update mypy * Bugfix to data normalization * Fix ref_key parsing * Update docs * Issue 92 (#98) * Base-line tests for future refactor. * Refactor to simplify code. * Implement issue #92 * Bump certifi from 2022.9.24 to 2022.12.7 (#95) Bumps [certifi](https://github.com/certifi/python-certifi) from 2022.9.24 to 2022.12.7. - [Release notes](https://github.com/certifi/python-certifi/releases) - [Commits](certifi/python-certifi@2022.09.24...2022.12.07) --- updated-dependencies: - dependency-name: certifi dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update version and changelog * update poetry lock * Update changelog * Issue 94 & 91 (#100) * Baseline unittests * Fix issue #94 * Remove test print * Fix issue #91 * Relax deepdiff dependency. * Update pyproject.toml Co-authored-by: Patryk Szulczewski <[email protected]> * Update poetry.lock * Relase v0.0.4 * Release v0.0.4 (#105) --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Christian Adell <[email protected]> Co-authored-by: Ken Celenza <[email protected]> Co-authored-by: Stephen Corry <[email protected]> Co-authored-by: Patryk Szulczewski <[email protected]> Co-authored-by: Federico87 <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Network to Code <[email protected]> Co-authored-by: Leo Kirchner <[email protected]> Co-authored-by: Leo Kirchner <[email protected]>

Fix parameter_match check, to support non-normalized data

2a026c7

pszulczewski requested review from chadell, lvrfrc87, scetron and jeffkala as code owners October 26, 2022 14:41

pszulczewski commented Oct 26, 2022

View reviewed changes

chadell reviewed Oct 27, 2022

View reviewed changes

Add test

5c28060

Patryk Szulczewski added 2 commits November 10, 2022 21:17

Update tests.

9ffda9a

remove test print

94b1c5e

lvrfrc87 reviewed Nov 11, 2022

View reviewed changes

lvrfrc87 self-requested a review November 15, 2022 09:26

lvrfrc87 merged commit 21428e8 into develop Nov 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix parameter_match check, to support non-normalized data #90

Fix parameter_match check, to support non-normalized data #90

pszulczewski commented Oct 26, 2022

pszulczewski Oct 26, 2022

pszulczewski Oct 26, 2022

lvrfrc87 Nov 11, 2022

chadell Oct 27, 2022

pszulczewski Oct 27, 2022

pszulczewski Oct 27, 2022 •

edited

Loading

lvrfrc87 Nov 11, 2022

lvrfrc87 commented Nov 10, 2022

pszulczewski commented Nov 10, 2022

lvrfrc87 left a comment

lvrfrc87 Nov 11, 2022

lvrfrc87 Nov 11, 2022

pszulczewski commented Nov 15, 2022

lvrfrc87 commented Nov 15, 2022

Fix parameter_match check, to support non-normalized data #90

Fix parameter_match check, to support non-normalized data #90

Conversation

pszulczewski commented Oct 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pszulczewski Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lvrfrc87 commented Nov 10, 2022

pszulczewski commented Nov 10, 2022

lvrfrc87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pszulczewski commented Nov 15, 2022

lvrfrc87 commented Nov 15, 2022

pszulczewski Oct 27, 2022 •

edited

Loading