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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 57 additions & 4 deletions sonic_cli_gen/yang_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,16 +259,17 @@ def on_uses(y_module: OrderedDict,
# trim prefixes in order to the next checks
trim_uses_prefixes(y_uses)

# TODO: 'refine' support
for group in y_grouping:
if isinstance(y_uses, list):
for use in y_uses:
if group.get('@name') == use.get('@name'):
resolve_refine(group, use)
ret_attrs.extend(get_leafs(group, group.get('@name')))
ret_attrs.extend(get_leaf_lists(group, group.get('@name')))
ret_attrs.extend(get_choices(y_module, group, conf_mgmt, group.get('@name')))
else:
if group.get('@name') == y_uses.get('@name'):
resolve_refine(group, y_uses)
ret_attrs.extend(get_leafs(group, group.get('@name')))
ret_attrs.extend(get_leaf_lists(group, group.get('@name')))
ret_attrs.extend(get_choices(y_module, group, conf_mgmt, group.get('@name')))
Expand Down Expand Up @@ -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.

return False

return False
return y_leaf.get('mandatory').get('@value') == 'true'


def get_description(y_entity: OrderedDict) -> str:
Expand Down Expand Up @@ -506,6 +507,58 @@ def get_uses(y_module: OrderedDict,
return []


def refine_cb(refine: OrderedDict, entity: OrderedDict):
if refine.get('@target-node') != entity.get('@name'):
return

if refine.get('description') is not None:
entity['description'] = OrderedDict([('text', refine.get('description').get('text'))])

if refine.get('mandatory') is not None:
entity['mandatory'] = OrderedDict([('@value', refine.get('mandatory').get('@value'))])


def resolve_refine_list(y_uses_refine: OrderedDict, y_entity: OrderedDict):
if isinstance(y_uses_refine, list):
if isinstance(y_entity, list):
for refine in y_uses_refine:
for e in y_entity:
refine_cb(refine, e)
else:
for refine in y_uses_refine:
refine_cb(refine, y_entity)
else:
if isinstance(y_entity, list):
for e in y_entity:
refine_cb(y_uses_refine, e)
else:
refine_cb(y_uses_refine, y_entity)


def resolve_refine(y_group: OrderedDict, y_uses: OrderedDict):
""" Check if the YANG 'uses' entity have the 'refine' entity, if so
this function will override values in 'y_group' using 'refine' values

Args:
y_group: reference to 'grouping'
y_uses: reference to 'uses'
"""

y_uses_refine = y_uses.get('refine')

if y_uses_refine is None:
return

y_group_leaf = y_group.get('leaf')
y_group_leaf_list = y_group.get('leaf-list')

if y_group_leaf is not None:
resolve_refine_list(y_uses_refine, y_group_leaf)

if y_group_leaf_list is not None:
resolve_refine_list(y_uses_refine, y_group_leaf_list)


def get_all_grouping(y_module: OrderedDict,
y_uses: OrderedDict,
conf_mgmt: ConfigMgmt) -> list:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -535,14 +535,14 @@
"attrs":[
{
"name":"GR_5_LEAF_1",
"description": "",
"description": "GR_5_LEAF_1 refine description",
"is-mandatory": False,
"is-leaf-list": False,
"group": "GR_5",
},
{
"name":"GR_5_LEAF_LIST_1",
"description": "",
"description": "GR_5_LEAF_LIST_1 refine description",
"is-mandatory": False,
"is-leaf-list": True,
"group": "GR_5",
Expand All @@ -556,7 +556,7 @@
},
{
"name":"GR_6_LEAF_2",
"description": "",
"description": "GR_6_LEAF_2 refine description",
"is-mandatory": False,
"is-leaf-list": False,
"group": "GR_6",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ module sonic-grouping-complex {

grouping GR_5 {
leaf GR_5_LEAF_1 {
mandatory true;
description "GR_5_LEAF_1 description";
type string;
}

Expand Down Expand Up @@ -87,8 +89,23 @@ module sonic-grouping-complex {

description "OBJECT_2 description";

uses GR_5;
uses GR_6;
uses GR_5 {
refine GR_5_LEAF_1 {
mandatory false;
description "GR_5_LEAF_1 refine description";
}

refine GR_5_LEAF_LIST_1 {
description "GR_5_LEAF_LIST_1 refine description";
}
}

uses GR_6 {
refine GR_6_LEAF_2 {
description "GR_6_LEAF_2 refine description";
}
}

uses sgroup2:GR_4;
}
}
Expand Down
Loading