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 Validation for MCLAG, NAT, MUXCABLE tables #2755

Merged
merged 7 commits into from
Apr 6, 2023

Conversation

isabelmsft
Copy link
Contributor

@isabelmsft isabelmsft commented Mar 23, 2023

What I did

Add YANG validation using GCU for writes to MCLAG, NAT, MUXCABLE tables tables in ConfigDB

How I did it

Using same method as https://github.com/sonic-net/sonic-utilities/pull/2190/files, extend to MCLAG, NAT, MUXCABLE tables

How to verify it

verified testing on virtual switch CLI

@qiluo-msft
Copy link
Contributor

Please add this PR to your HLD PR's description code PR list, so people can find relevant HLD.

except ValueError as e:
ctx.fail("Invalid ConfigDB. Error: {}".format(e))
else:
ctx.fail("only one mclag Domain can be configured. Already one domain {} configured ".format(list(mclag_domain_keys)[0]))
Copy link
Contributor

@qiluo-msft qiluo-msft Mar 30, 2023

Choose a reason for hiding this comment

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

list

Just wondering whether this is a bug fix? #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.

Yes. Without the change, we get this error

  File "/usr/local/lib/python3.9/dist-packages/config/mclag.py", line 161, in add_mclag_domain
    print(mclag_domain_keys[0])
TypeError: 'dict_keys' object is not subscriptable 

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for your contribution to fix a bug in codebase!

config/nat.py Outdated

#
# 'nat add binding' command ('config nat add binding <binding_name> <pool_name> <acl_name>')
# 'nat add binding' command ('config nat add binding <ninding_name> <pool_name> <acl_name>')
Copy link
Contributor

@qiluo-msft qiluo-msft Mar 31, 2023

Choose a reason for hiding this comment

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

ninding_name

typo? #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.

yes, corrected the typo now

config/console.py Outdated Show resolved Hide resolved
import ipaddress

import jsonpatch
from jsonpatch import JsonPatchConflict
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use the same import method?
"import jsonpatch" or "from jsonpatch import xxx"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cleaned import statement

import jsonpatch
import jsonpointer
from jsonpatch import JsonPatchConflict
from jsonpointer import JsonPointerException
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the same import method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cleaned import statements

config/nat.py Show resolved Hide resolved
@vaibhavhd vaibhavhd requested a review from vdahiya12 April 5, 2023 19:31
@isabelmsft isabelmsft merged commit eab5466 into sonic-net:master Apr 6, 2023
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.

3 participants