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

Rectify buffer_port_profile(ing/egr) yang files & related tests #10334

Closed

Conversation

AmitKaushik7
Copy link
Contributor

Why I did it

Rectified Buffer Port Ingress/Egress Profile yang files derived from sonic/mgmt-cvl/testdata/schema from sonic-mgmt-common, used by mgmt-framework.

Updated BUFFER_PORT_INGRESS_PROFILE_LIST and BUFFER_PORT_EGRESS_PROFILE_LIST - Use Comma Separated string as profile list instead of List of Profile.

How I did it

     "BUFFER_PORT_EGRESS_PROFILE_LIST": {
         "Ethernet9": {
  •            "profile_list": ["egress_lossless_profile", "egress_lossy_profile"]   <== Removed
    
  •            "profile_list": "egress_lossless_profile, egress_lossy_profile"     <== Added
           }
       },
    

Same change for BUFFER_PORT_INGRESS_PROFILE_LIST.

How to verify it

Rectified the test cases for ingress/egress buffer profile list.
test_yang_model.py 210 INFO BUFFER_PORT_INGRESS_PROFILE_LIST_CORRECT_PROFILE_VALUE no failure Passed
test_yang_model.py 210 INFO BUFFER_PORT_EGRESS_PROFILE_LIST_WRONG_PORT_VALUE pattern failure Passed
test_yang_model.py 210 INFO BUFFER_PORT_EGRESS_PROFILE_LIST_CORRECT_PROFILE_VALUE no failure Passed
test_yang_model.py 210 INFO BUFFER_PORT_EGRESS_PROFILE_LIST_MANDATORY_PROFILE_VALUE no profile Passed
test_yang_model.py 210 INFO BUFFER_PORT_INGRESS_PROFILE_LIST_MANDATORY_PROFILE_VALUE no profile Passed
test_yang_model.py 210 INFO BUFFER_PORT_INGRESS_PROFILE_LIST_WRONG_PORT_VALUE pattern failure Passed

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

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

Description for the changelog

Rectify buffer_port_profile(ing/egr) yang files & update related test cases.

Link to config_db schema for YANG module changes

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

@qiluo-msft
Copy link
Collaborator

@Junchao-Mellanox Please help review.

@Junchao-Mellanox
Copy link
Collaborator

Junchao-Mellanox commented Mar 24, 2022

The old yang definition looks better. Could you please elaborate the reason to change it from list to string? If it is for the DPB issue, there is already a PR to handle it: #10291

@AmitKaushik7
Copy link
Contributor Author

I have not heard back on my comments last week in PR#9801, hence raised this PR.
If you agree that old yang model is correct, then I will cancel this PR.

@dgsudharsan
Copy link
Collaborator

dgsudharsan commented Mar 24, 2022

@AmitKaushik7 The suggestion from the yang community is to first initially fix it in yang infra and then slowly correct the offending configurations and not the yang model itself. Please sync up with @bandaru-viswanath

@AmitKaushik7
Copy link
Contributor Author

Ok. closing the PR.

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.

4 participants