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

[yang] add Yang model for GRPCCLIENT #20902

Merged
merged 4 commits into from
Nov 26, 2024

Conversation

vdahiya12
Copy link
Contributor

@vdahiya12 vdahiya12 commented Nov 23, 2024

Why I did it

Adding yang model for CONFIG_DB table GRPCCLIENT
Introduced by
https://github.com/sonic-net/sonic-platform-daemons/blob/b276e415d85f3e8a7c64532d95821c9000c9418d/sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py#L407

sign-off: Vaibhav Dahiya [vdahiya@microsoft.com]

Work item tracking
  • Microsoft ADO (number only):
    30350460

How I did it

How to verify it

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@wen587 wen587 requested review from wen587 and ganglyu November 25, 2024 09:33

container GRPCCLIENT {

container CONFIG {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is case sensitive. Need to put them lowercase to validate yang. otherwise will fail:

sonic_yang(3):All Keys are not parsed in GRPCCLIENT
dict_keys(['certs', 'config'])
sonic_yang(3):exceptionList:[]
sonic_yang(3):Data Loading Failed:All Keys are not parsed in GRPCCLIENT
dict_keys(['certs', 'config'])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

}
}

container CERTS {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above. case sensitive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


type string;
description "grpc client root cert";

Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove some of newline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

"GRPCCLIENT_CONFIG_CHANGE_VERBOSITY_LEVEL": {
"sonic-grpcclient:sonic-grpcclient": {
"sonic-grpcclient:GRPCCLIENT": {
"sonic-grpcclient:CONFIG":
Copy link
Contributor

Choose a reason for hiding this comment

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

CONFIG should be config which should be same as
sample_config_db.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
Copy link
Contributor

@wen587 wen587 left a comment

Choose a reason for hiding this comment

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

lgtm

@vdahiya12
Copy link
Contributor Author

@qiluo-msft please help merge

@qiluo-msft qiluo-msft merged commit 8d016e7 into sonic-net:master Nov 26, 2024
22 checks passed
@bingwang-ms
Copy link
Contributor

@vdahiya12 Please raise a PR to 202405 to address conflict.

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.

6 participants