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

Fix sibling check for lists. #54

Merged
merged 5 commits into from
Nov 3, 2023
Merged

Fix sibling check for lists. #54

merged 5 commits into from
Nov 3, 2023

Conversation

wenovus
Copy link
Contributor

@wenovus wenovus commented Nov 3, 2023

The existing check does not work if some of the nodes are augmented into the surrounding container (see openconfig/public#991).

  • Removed old check and added new check.
  • Added tests for both cases.
  • Added a nil check for check_list_enclosing_container.

Tested at openconfig/public#993

The existing check does not work if some of the nodes are augmented into
the surrounding container.

Removed old check and added new check. Added tests for both cases.
Copy link
Contributor

@robshakir robshakir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this. I had assumed that pyang gave us the augmented tree in this case.

@wenovus
Copy link
Contributor Author

wenovus commented Nov 3, 2023

Thanks for catching this. I had assumed that pyang gave us the augmented tree in this case.

This could be a pyang issue depending on the original intent. For some reason the augmented node only appears when validating the augmented nodes. They are validated at the end of reference_2 and prior to it the surrounding container contained only the original list.

@wenovus wenovus merged commit 4607fd1 into master Nov 3, 2023
4 checks passed
@wenovus wenovus deleted the fix-lists-no-sibling-check branch November 3, 2023 22:02
@wenovus
Copy link
Contributor Author

wenovus commented Nov 3, 2023

Filed an issue in pyang: mbj4668/pyang#881

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 this pull request may close these issues.

3 participants