Skip to content

Conversation

@alexandernorth
Copy link
Contributor

This PR includes several fixes:

  • (legacy) fixes to netconf xml generation pre-tree
  • Added sort option to tree XML generation to order tree based on schema keys (options: None, Alphabetical, SchemaBound)

@steiler
Copy link
Collaborator

steiler commented Nov 21, 2024

Thanks Alex!
Can I ask what the purpose for the SchemaBound ordering is? Unfortunately I do not understand it. Thanks

@alexandernorth
Copy link
Contributor Author

alexandernorth commented Nov 21, 2024

Of course, it came from the requirement that our NF seems to require list elements with a key defined to have the first children in the order defined in that key, which may not be the alphabetical order.
For example, given the following Yang model:

list a {
    key “id”;
    leaf id {}
    leaf abc {}
}

In the current sort implementation, abc would be the first child and before id which would cause the config to be rejected by the NF.

SchemaBound takes this key priority ordering into account when generating the XML structure, placing them in the correct order (according to the schema) first, then sorting the remaining non-key children alphabetically.

This PR actually should depend on sdcio/schema-server#134 which fixes the order of the keys sent by the schema-server to match the order given in the Yang model.

@steiler
Copy link
Collaborator

steiler commented Nov 21, 2024

You seem to be totally right...

Key elements must be the first elements within the list entry to comply with the YANG and NETCONF standards.
we were also discussing if we should add more option flags or simply go with the sorted output... now also given this, I tend to do so.
The other discussion point was to also take data types into account and sort not just in alphabetical (string) order.
So if you like and have the time, give it a shot maybe.

@alexandernorth
Copy link
Contributor Author

The other discussion point was to also take data types into account and sort not just in alphabetical (string) order.
So if you like and have the time, give it a shot maybe.

Happy to look into this, could you give an example of what your idea is? I am not sure how the data type would be used to define the order of the list children

@steiler
Copy link
Collaborator

steiler commented Nov 21, 2024

right now we're using tvToString and then do use tring order...
1
10
13
2
25

to lets take the type into account and convert (u)int(8,16,32) to (u)ints and not use string comparison.

@hansthienpondt
Copy link
Contributor

hansthienpondt commented Dec 6, 2024

==============================================================================
02-Crud                                                               | PASS |
117 tests, 117 passed, 0 failed
==============================================================================

i will approve schema-server and bump the version; then will update this PR with the adjusted version

Copy link
Contributor

@hansthienpondt hansthienpondt left a comment

Choose a reason for hiding this comment

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

LGTM

@hansthienpondt hansthienpondt merged commit 0a56c3a into sdcio:main Dec 9, 2024
1 check passed
@alexandernorth alexandernorth deleted the fix/data-manipulation branch December 9, 2024 12:38
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