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

Fixed issue "CRM module throws error". #543

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixed issue "CRM module throws error". #543

wants to merge 1 commit into from

Conversation

iris00522
Copy link

  • What I did
    If we input crm thresholds parameters incompletely, it cause crm module throws error while accessing crm config info.
  • How I did it
    The parameters which aren't set display with default value.(reference sonic-swss/orchagent/crmorch.h)
  • How to verify it
    Config partial thresholds parameters.
    Execute CRM show commands and check it not cause crm module throws error.

@prsunny
Copy link
Contributor

prsunny commented May 28, 2019

I think this check and the values (out-of-range) should be done by the config validator. I would prefer CLI to provide the expected arguments.

@iris00522
Copy link
Author

iris00522 commented May 29, 2019

If i perform the cmd
"crm config thresholds acl group high 300
"crm config thresholds acl group type percentage"
It lose config the thresholds acl group low
The table just show the value, witch it can get.
or don't show anything.
For example.

1.root@sonic:~# crm show thresholds acl group
Resource Name Threshold Type Low Threshold High Threshold


acl_group percentage x 300
x means could not get CRM configuration.

2.root@sonic:~# crm show thresholds acl group
Resource Name Threshold Type Low Threshold High Threshold


which one would prefer?

@gord1306
Copy link
Contributor

@prsunny , the issue seems to be the crm_info only get value from configDB, and it cannot get the default value from crmorch. If no value in config DB, the content crm_info will be incomplete for the crm show command.

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.

None yet

3 participants