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

Fix Wildcard entries to return empty data instead of invalid path #299

Merged
merged 15 commits into from
Nov 11, 2024

Conversation

zbud-msft
Copy link
Contributor

Why I did it

How I did it

How to verify it

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Link to config_db schema for YANG module changes

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

@zbud-msft zbud-msft marked this pull request as ready for review October 11, 2024 23:00
@@ -3487,6 +3487,77 @@ func TestClientConnections(t *testing.T) {
}
}

func TestNonExistentTableNoError(t *testing.T) {
Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 17, 2024

Choose a reason for hiding this comment

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

TestNonExistentTableNoError

Is there a test case for existing table no error? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added test

qiluo-msft
qiluo-msft previously approved these changes Oct 17, 2024
xincunli-sonic
xincunli-sonic previously approved these changes Oct 18, 2024
ganglyu
ganglyu previously approved these changes Oct 18, 2024
FengPan-Frank
FengPan-Frank previously approved these changes Oct 18, 2024

if diff := pretty.Compare(tt.wantNoti, gotNoti); diff != "" {
t.Log("\n Want: \n", tt.wantNoti)
t.Log("\n Got: \n", gotNoti)
Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 23, 2024

Choose a reason for hiding this comment

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

We also need to verify if the gnmi response is correct.
Seems like a bug in your manual test: "{}" #Closed

Copy link
Contributor Author

@zbud-msft zbud-msft Nov 5, 2024

Choose a reason for hiding this comment

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

Added test to verify gnmi response is empty json, please take a look at EMPTY_JSON.txt.

It is not a bug, as gnmi_client as part of subscribe will print out json_ietf_value as json string and "{}" is a valid response.

json_ietf structured value should be a valid json string, which empty json represented as string, "{}", is.
https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-specification.md#23-structured-data-types

If we use gnmi_get which prints out json and not json string, we indeed get expected empty json
[elem {
name: "PSU_INFO"
}
]
The GetResponse is below

{}

@gechiang
Copy link

@qiluo-msft , @zbud-msft has made the changes per your review comment. if the new changes looks good and no other comments, can you you help approve/merge this PR?
Thanks!

@gechiang gechiang requested a review from BYGX-wcr November 11, 2024 21:29
@gechiang
Copy link

@BYGX-wcr , please help monitor this PR state. When it is merged, please work with @zbud-msft to port this needed fix to internal 202205 chassis branch via patch. Thanks!

@gechiang gechiang merged commit e784b6f into sonic-net:master Nov 11, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants