Skip to content
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

jdiff not ble to catch reference key in dict of dicts #91

Closed
lvrfrc87 opened this issue Nov 11, 2022 · 5 comments · Fixed by #100
Closed

jdiff not ble to catch reference key in dict of dicts #91

lvrfrc87 opened this issue Nov 11, 2022 · 5 comments · Fixed by #100
Assignees

Comments

@lvrfrc87
Copy link
Collaborator

lvrfrc87 commented Nov 11, 2022

Environment

  • Python version: 3.7
  • jdiff version: 0.0.2

Expected Behavior

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

@lvrfrc87
Copy link
Collaborator Author

I believe this could be a limitation from JMSPATH. The most similar case I found is this one:https://jmespath.org/tutorial.html#object-projections

As we can see, the our hypothetical reference key is lost using *. I will drop a message on jmspath channel support but I feel that the user will have to do some post parsing using indexes

@lvrfrc87
Copy link
Collaborator Author

lvrfrc87 commented Nov 15, 2022

Looks like there is keys(@) which catches all dict keys. So in jdiff the path should be something like $keys(@)$.is_up and should work. @pszulczewski FYI

Screenshot 2022-11-15 at 10 25 43

@pszulczewski
Copy link
Collaborator

keys(@) produce list of keys, similar to python dict.keys(), they don't have access to values.

Thus keys(@).is_up or $keys(@)$.is_up return None.

@lvrfrc87
Copy link
Collaborator Author

At this point we have 3 options:

  • Let the user use index to create a mapping between key and value got from jdiff - this will not work out of the box with pre defined checks
  • Make jdiff dynamically discovery the data type and act accordingly - for this case the expression would be something like $*$.is_up
  • Create some sort of flag

@lvrfrc87 lvrfrc87 self-assigned this Nov 17, 2022
@lvrfrc87
Copy link
Collaborator Author

As discussed with @pszulczewski we will support $*$.is_up and interpolate the list of keys with the list of values returned.

pszulczewski added a commit that referenced this issue Apr 20, 2023
@pszulczewski pszulczewski linked a pull request Apr 20, 2023 that will close this issue
lvrfrc87 pushed a commit that referenced this issue Apr 20, 2023
* Baseline unittests

* Fix issue #94

* Remove test print

* Fix issue #91
lvrfrc87 added a commit that referenced this issue May 12, 2023
* 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]>
lvrfrc87 added a commit that referenced this issue May 12, 2023
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants