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

[sonic_cli_gen] Add YANG refine support #2804

Conversation

vadymhlushko-mlnx
Copy link
Contributor

What I did

Add YANG "refine" support for the sonic_cli_gen utility.
YANG refine is used to override the description and mandatory statements for YANG grouping.

How I did it

Modify the sonic_cli_gen/yang_parser.py

How to verify it

Modify the sonic_cli_gen UT

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@liat-grozovik
Copy link
Collaborator

@qiluo-msft could you please help to review?

@liat-grozovik
Copy link
Collaborator

@qiluo-msft kindly reminder to review.

@liat-grozovik
Copy link
Collaborator

@qiluo-msft any comments or we can go a head and merge?

@@ -401,10 +402,10 @@ def get_mandatory(y_leaf: OrderedDict) -> bool:
'leaf' 'mandatory' value
"""

if y_leaf.get('mandatory') is not None:
return True
if y_leaf.get('mandatory') is None:
Copy link
Contributor

@qiluo-msft qiluo-msft Jun 29, 2023

Choose a reason for hiding this comment

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

None

Thanks for this bug fix! Could you separate it into a standalone PR? #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a bug fix.
I've changed the logic of this function because before YANG refine support feature extension the YANG leaf[mandatory] statement has only the true value OR YANG leaf[mandatory] doesn't exist at all

Copy link

@praveen-li praveen-li Jul 1, 2023

Choose a reason for hiding this comment

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

Logic of both code is same.
Though, assume we have to wrap it in try-catch. In that case, what should be return value from catch block i.e. in exception case?

-- If false then current block is good.
-- If true then new block is good.

I prefer, false, because by default, each node is non mandatory.

@zhangyanzhao zhangyanzhao force-pushed the master-sonic-cli-gen-refine-support branch from f37bd24 to ad4776f Compare July 13, 2023 17:26
@zhangyanzhao
Copy link
Collaborator

@qiluo-msft will further check and merge. Thanks.

@keboliu
Copy link
Collaborator

keboliu commented Jul 25, 2023

@qiluo-msft since it has been approved, would you please help to merge it.

@qiluo-msft qiluo-msft merged commit 5ae17ff into sonic-net:master Jul 27, 2023
4 checks passed
@StormLiangMS
Copy link
Contributor

enhancement, no for cherry-pick

pdhruv-marvell pushed a commit to pdhruv-marvell/sonic-utilities that referenced this pull request Aug 23, 2023
#### What I did
Add [YANG "refine"](https://www.rfc-editor.org/rfc/rfc7950.html#section-7.13.2) support for the `sonic_cli_gen` utility.
YANG refine is used to override the `description` and `mandatory` statements for YANG `grouping`.

#### How I did it
Modify the `sonic_cli_gen/yang_parser.py`

#### How to verify it
Modify the `sonic_cli_gen` UT
@vadymhlushko-mlnx
Copy link
Contributor Author

@StormLiangMS it should be cherry-picked to the 202305 because current YANG models use the refine keyword

@dgsudharsan
Copy link
Collaborator

@StormLiangMS This is not a feature enhancement but required for yang models that are backported as fix to use refine keyword. Please help to cherry-pick

@dgsudharsan
Copy link
Collaborator

@StormLiangMS This change was not cherry-picked to 202305 but the label was added. Can you please check?

StormLiangMS pushed a commit that referenced this pull request Oct 18, 2023
#### What I did
Add [YANG "refine"](https://www.rfc-editor.org/rfc/rfc7950.html#section-7.13.2) support for the `sonic_cli_gen` utility.
YANG refine is used to override the `description` and `mandatory` statements for YANG `grouping`.

#### How I did it
Modify the `sonic_cli_gen/yang_parser.py`

#### How to verify it
Modify the `sonic_cli_gen` UT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants