Plugin SID bugfixes and improvements#945
Conversation
|
I will add a note that the |
|
Does this reflect the consensus on the yang-tooling list? The PR has conflicts, can you fix them please? |
Partly. There are problems with base RFC 9595 document. The problems are being (slowly) resolved on the CoRE WG mailing list, but we have not yet reached consensus. On the other hand the changes included within the PR are things that are handled wrong by PYANG and will not be changed by the WG. From this point of view, yes this PR reflect yang-tooling consensus (even the CoRE WG consensus). @abierman Do you agree that the changes are consensus based?
Conflicts against what branch? The GitHub is telling me that there are not conflicts found. |
|
Hi,
I do not think yang-tooling is aware of this PR.
The SID plugin author needs to review these changes.
Andy
…On Thu, Mar 12, 2026 at 3:53 PM vvilimek ***@***.***> wrote:
*vvilimek* left a comment (mbj4668/pyang#945)
<#945 (comment)>
Does this reflect the consensus on the yang-tooling list?
Partly. There are problems with base RFC 9595 document. The problems are
being (slowly) resolved on the CoRE WG mailing list, but we have not yet
reached consensus. On the other hand the changes included within the PR are
things that are handled wrong by PYANG and will not be changed by the WG.
From this point of view, yes this PR reflect yang-tooling consensus (even
the CoRE WG consensus).
@abierman <https://github.com/abierman> Do you agree that the changes are
consensus based?
The PR has conflicts, can you fix them please?
Conflicts against what branch? The GitHub is telling me that there are not
conflicts found.
—
Reply to this email directly, view it on GitHub
<#945 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAN2BD63Z45SLLURXPM2UDL4QM5YFAVCNFSM6AAAAACV3QEPMKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DANJQG44DCOJUGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I will fix that. |
|
Nobody objected the changes. I wrote on the yang-tooling ML on 2026-03-13. I will call it a ML consensus. The messages in threads are not closely related (mostly). Initial ML message can be found here. |
When I look at the PR in GitHub it says "This branch cannot be rebased due to conflicts". |
I wil fix them.
What is the base branch for rebasing? |
Commit also contains tests for new functionality.
The RFC 9595 uses singular form for name of list data nodes. Tests are changed accordingly.
What do you mean with base branch? This is "merge 27 commits into |
|
Sorry, when you only merge the changes, it is clean GH: "No conflicts with base branch" and "Changes can be cleanly merged.". But when you are rebasing the |
We add the mandatory 'ietf-sid-file:sid-file' toplevel JSON object. We also implement the field 'sid-file-status'. We reorder the 'sid-file' fields to reflect the YANG model definition and RFC 9595 example .sid file order. TODOs: - implement sid-file-version - implement cli option for description - improve the sid-file-status to reflect status of 'item' list
We want to use similar naming convention to YANG module filenames.
The previous version generated paths incorrectly because the submodules does not have their own namespace. Submodules live in namespace of it's parent module. For example (for module ietf-snmp.yang and all it's submodules) /ietf-snmp-common:snmp/ietf-snmp-engine:engine/engine-id is incorrect /ietf-snmp:snmp/engine/engine-id is correct
Fix issue with duplicit same module with same or different revision in the 'dependency-revision' list. The 'dependency-revision' list is now correctly sorted.
I wrongly removed generation of items of submodules. This commit reverts my mistake. Thanks to Andy Bierman for notice.
3 participants
Summary of changes:
Reported by Andy Bierman on IETF CoRE mailing list. The initial report is here.
.sidfiles for YANG submodules.Related to previous point.
.sidfile indicating it was created by pyang including the generation timestamp in UTC.A nice-to-have also mentioned by Andy Bierman, see issue on CoRE fork. The issue is based on CoRE mailing list message.
'dependency-revision'list during.sidfile update by properly unifying entries.Reported by Michael Richardson, see issue on CoRE fork.
Thanks all for reporting. I hope the code has a decent quality. Tests are included.