Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[crm] add CLI validation for CRM polling interval range #2293

Merged

Conversation

ayurkiv-nvda
Copy link
Contributor

@ayurkiv-nvda ayurkiv-nvda commented Aug 3, 2022

Signed-off-by: Andriy Yurkiv ayurkiv@nvidia.com

What I did

add checking for range for CRM interval

How I did it

add attribute click.IntRange(1, 9999) and UT to verify it (according to CRM HLD)

How to verify it

  1. run UT
  2. manual testing: crm config polling interval 100000000 (receive error)

Previous command output (if the output of a command-line utility has changed)

crm config polling interval 4566466
crm config polling interval --help
Usage: crm config polling interval [OPTIONS] INTERVAL

  CRM polling interval configuration

Options:
  --help  Show this message and exit.

New command output (if the output of a command-line utility has changed)

crm config polling interval 4566466
Usage: crm config polling interval [OPTIONS] INTERVAL
Try "crm config polling interval --help" for help.

Error: Invalid value for "INTERVAL": 4566466 is not in the valid range of 1 to 9999.

crm config polling interval --help
Usage: crm config polling interval [OPTIONS] INTERVAL

  CRM polling interval configuration in seconds (range: 1-9999)

Options:
  --help  Show this message and exit.

Signed-off-by: Andriy Yurkiv <ayurkiv@nvidia.com>
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 3, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: ayurkiv-nvda / name: Andriy Yurkiv (88109cf)

@liat-grozovik liat-grozovik merged commit ca14133 into sonic-net:master Aug 8, 2022
@ayurkiv-nvda ayurkiv-nvda deleted the crm_interval_range_azure branch August 8, 2022 20:38
@ayurkiv-nvda ayurkiv-nvda restored the crm_interval_range_azure branch August 8, 2022 20:38
yxieca pushed a commit that referenced this pull request Aug 8, 2022
- What I did
Add checking for range for CRM interval

- How I did it
Add attribute click.IntRange(1, 9999) and UT to verify it (according to CRM HLD)

- How to verify it
Run UT
Manual testing: crm config polling interval 100000000 (receive error)

- Previous command output (if the output of a command-line utility has changed)
crm config polling interval 4566466
crm config polling interval --help
Usage: crm config polling interval [OPTIONS] INTERVAL

  CRM polling interval configuration

Options:
  --help  Show this message and exit.

- New command output (if the output of a command-line utility has changed)
crm config polling interval 4566466
Usage: crm config polling interval [OPTIONS] INTERVAL
Try "crm config polling interval --help" for help.

Error: Invalid value for "INTERVAL": 4566466 is not in the valid range of 1 to 9999.

crm config polling interval --help
Usage: crm config polling interval [OPTIONS] INTERVAL

  CRM polling interval configuration in seconds (range: 1-9999)

Options:
  --help  Show this message and exit.

Signed-off-by: Andriy Yurkiv <ayurkiv@nvidia.com>
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Aug 9, 2022
Update sonic-utilities submodule pointer to include the following:
* Fix GCU bug when backend service modifying config ([sonic-net#2295](sonic-net/sonic-utilities#2295))
* Fix issues for sonic_installer upgrade-docker and sonic_installer rollback-docker ([sonic-net#2278](sonic-net/sonic-utilities#2278))
* [crm] add checking for CRM interval range ([sonic-net#2293](sonic-net/sonic-utilities#2293))
* Fix the issue that sonic_platform is not installed on vs image ([sonic-net#2300](sonic-net/sonic-utilities#2300))
* Add FEC correctable and uncorrectable port stats ([sonic-net#2027](sonic-net/sonic-utilities#2027))
* Add CLI to configure YANG config validation ([sonic-net#2147](sonic-net/sonic-utilities#2147))
* Add override testcase to verify removal ([sonic-net#2288](sonic-net/sonic-utilities#2288))
* Fix version in db_migrator  for  ([sonic-net#2289](sonic-net/sonic-utilities#2289))
* [intfutil] Check whether the FEC mode is supported on the platform before configuring it to CONFIG_DB ([sonic-net#2223](sonic-net/sonic-utilities#2223))
* Transfer organization from Azure to sonic-net ([sonic-net#2284](sonic-net/sonic-utilities#2284))
* [watermarkstat] Fix CLI script for unconfigured PG counters ([sonic-net#2239](sonic-net/sonic-utilities#2239))
* Improve the way to check port type of RJ45 port ([sonic-net#2249](sonic-net/sonic-utilities#2249))

Signed-off-by: dprital <drorp@nvidia.com>
preetham-singh pushed a commit to preetham-singh/sonic-utilities that referenced this pull request Nov 21, 2022
- What I did
Add checking for range for CRM interval

- How I did it
Add attribute click.IntRange(1, 9999) and UT to verify it (according to CRM HLD)

- How to verify it
Run UT
Manual testing: crm config polling interval 100000000 (receive error)

- Previous command output (if the output of a command-line utility has changed)
crm config polling interval 4566466
crm config polling interval --help
Usage: crm config polling interval [OPTIONS] INTERVAL

  CRM polling interval configuration

Options:
  --help  Show this message and exit.

- New command output (if the output of a command-line utility has changed)
crm config polling interval 4566466
Usage: crm config polling interval [OPTIONS] INTERVAL
Try "crm config polling interval --help" for help.

Error: Invalid value for "INTERVAL": 4566466 is not in the valid range of 1 to 9999.

crm config polling interval --help
Usage: crm config polling interval [OPTIONS] INTERVAL

  CRM polling interval configuration in seconds (range: 1-9999)

Options:
  --help  Show this message and exit.

Signed-off-by: Andriy Yurkiv <ayurkiv@nvidia.com>
@ayurkiv-nvda ayurkiv-nvda deleted the crm_interval_range_azure branch February 5, 2025 10:40
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants