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

[GCU] Add PORT table StateDB Validator #2936

Merged
merged 11 commits into from
Aug 16, 2023

Conversation

isabelmsft
Copy link
Contributor

@isabelmsft isabelmsft commented Aug 2, 2023

Feature ADO 24806590
Test Case Bug Fix ADO 26158250

What I did

Add PORT table StateDB Validator

How I did it

Add new validating function for PORT table field and fec fields

How to verify it

Unit tests, updated sonic-mgmt tests: https://github.com/sonic-net/sonic-mgmt/pull/9403/files

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)

@isabelmsft isabelmsft marked this pull request as ready for review August 5, 2023 07:30


def read_statedb_entry(table, field):
state_db = SonicV2Connector(host="127.0.0.1")
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 11, 2023

Choose a reason for hiding this comment

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

SonicV2Connector

If you use Table class, you do not need to concat table name and key. #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.

I see.. is Table class recommended over SonicV2Connector use_unix_socket_path? In the future, this function may be extended to include tables other than PORT

Copy link
Contributor

Choose a reason for hiding this comment

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

As I said, the benefit is no need to concat table name/separator/key. You can create multiple Table from one DBConnector, and they are not heavy.

if key == "speed":
supported_speeds_str = read_statedb_entry('{}|{}'.format("PORT_TABLE", port), "supported_speeds") or ''
supported_speeds = [int(s) for s in supported_speeds_str.split(',') if s]
if supported_speeds and int(value) not in supported_speeds:
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 11, 2023

Choose a reason for hiding this comment

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

int

may throw during casting #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.

Thank you, I added exception handling

patch_element_str = json.dumps(patch_element)
path = patch_element["path"]
value = patch_element.get("value")
keys = ['fec', 'speed']
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 11, 2023

Choose a reason for hiding this comment

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

keys

-> fields #Closed

@@ -145,6 +145,9 @@
}
}
}
},
"PORT": {
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 11, 2023

Choose a reason for hiding this comment

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

mixing tab and spaces. #Closed

@isabelmsft isabelmsft merged commit 8cb7320 into sonic-net:master Aug 16, 2023
4 checks passed
@isabelmsft
Copy link
Contributor Author

Tested on 202305 branch, confirmed no cherry-pick conflicts

mssonicbld pushed a commit to mssonicbld/sonic-utilities that referenced this pull request Dec 21, 2023
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #3096

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.

5 participants