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

Feature yang changes #7955

Merged
merged 6 commits into from
Oct 5, 2021
Merged

Feature yang changes #7955

merged 6 commits into from
Oct 5, 2021

Conversation

arlakshm
Copy link
Contributor

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan [email protected]

Why I did it

Add yang model for Feature configuration

How I did it

Add feature.yang and unit tests

How to verify it

Compile target/python-wheels/sonic_yang_models-1.0-py3-none-any.whl

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • [x ] 202012

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
@anshuv-mfst anshuv-mfst added the YANG YANG model related changes label Jun 23, 2021
@zhangyanzhao
Copy link
Collaborator

not ready for review, need move it back to "To be Reviewed column"

@arlakshm arlakshm marked this pull request as ready for review September 8, 2021 20:29
@arlakshm arlakshm requested a review from lguohan as a code owner September 8, 2021 20:29
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
@zhangyanzhao
Copy link
Collaborator

Arvind will handle the comments from Venkat.

@venkatmahalingam
Copy link
Collaborator

@arlakshm In the mgmt VRF YANG, boolean value is represented as "true" in the below file, please try this for your case as well.
https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/tests/files/sample_config_db.json#L281

@zhangyanzhao
Copy link
Collaborator

@arlakshm please address the comments from Venkat. Thanks.

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
@arlakshm
Copy link
Contributor Author

@arlakshm In the mgmt VRF YANG, boolean value is represented as "true" in the below file, please try this for your case as well.
https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/tests/files/sample_config_db.json#L281

The backend python code expects "True" or "False" I think change to "true" may not work.

@venkatmahalingam
Copy link
Collaborator

venkatmahalingam commented Sep 30, 2021

@arlakshm In the mgmt VRF YANG, boolean value is represented as "true" in the below file, please try this for your case as well.
https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/tests/files/sample_config_db.json#L281

The backend python code expects "True" or "False" I think change to "true" may not work.

boolean type is represented as "true/"false" in the table (MGMT VRF, LLDP..etc) fields, can we change the backend to accept "true"/"false" to be consistent.

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <[email protected]>
@arlakshm arlakshm requested a review from abdosi October 5, 2021 22:12
leaf high_mem_alert {
description "This configuration controls the trigger to generate
alert on high memory utilization";
type stypes:feature_state;
Copy link
Contributor

Choose a reason for hiding this comment

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

@arlakshm does always_enabled applies for high_mem_alert ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@arlakshm does always_enabled applies for high_mem_alert ?

I think it is OK to let high_mem_alert support the option always_enabled.

@arlakshm arlakshm merged commit 3426739 into sonic-net:master Oct 5, 2021
@arlakshm arlakshm deleted the feature_yang branch October 5, 2021 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
YANG YANG model related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants