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

[config] Fix some issues in config. #2416

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

LGH-12
Copy link

@LGH-12 LGH-12 commented Sep 30, 2022

Signed-off-by: LTeng liuteng@asterfusion.com

What I did

Fix #2415
1、At config/config_mgmt.py:648, there is "Exce" which looks like it should be "Exception".
2、Cmd "config mirror_session erspan add": gre_type is not required. validate_gre_type raise " AttributeError: 'NoneType' object has no attribute 'lower' " if gre_type is not provided.
3、Cmd "config interface cable_length": It is actually "config interface cable-length" at running time and caused by click 7.0 . And It is inconsistent with Command-Reference.
4、Cmd "config route del": In "if not tuple(key.split("|")) in keys and not prefix_tuple in keys", these two look the same.
5、Cmd "config sflow polling-interval": echo error but no return.

How I did it

1、Replace "Exce" with "Exception".
2、Add check: value is not None.
3、assign the name explicitly.Modified comments of priority-group for the same reason.
4、Remove one of them.
5、Replace "click.echo" with "ctx.fail".

How to verify it

Added unit tests.

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)

LGH-12 and others added 4 commits September 30, 2022 14:52
Signed-off-by: LTeng <liuteng@asterfusion.com>
Signed-off-by: LTeng <liuteng@asterfusion.com>
Signed-off-by: LTeng <liuteng@asterfusion.com>
config/main.py Outdated
@@ -4717,7 +4717,7 @@ def remove_queue(db, interface_name, queue_map):
# 'cable_length' subcommand
#

@interface.command()
@interface.command("cable_length")
Copy link
Contributor

Choose a reason for hiding this comment

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

"cable_length"

cable-length

Copy link
Author

Choose a reason for hiding this comment

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

done !

@maipbui
Copy link
Contributor

maipbui commented Jun 29, 2023

Please resolve conflicts and merge latest master code to trigger Semgrep.

@LGH-12 LGH-12 requested a review from maipbui July 2, 2023 10:38
@maipbui
Copy link
Contributor

maipbui commented Jul 3, 2023

Please add unit test to cover this change.

LGH-12 added 3 commits July 7, 2023 11:30
Signed-off-by: LTeng <liuteng@asterfusion.com>
Signed-off-by: LTeng <liuteng@asterfusion.com>
@LGH-12
Copy link
Author

LGH-12 commented Jul 7, 2023

Please add unit test to cover this change.

Updated !

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.

[config] Several issue records
2 participants