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

new commands to add a range of vlans and add a member to a range of vlans #891

Closed
wants to merge 8 commits into from

Conversation

anilkpan
Copy link
Contributor

- What I did
Added new commands to be able to add a range of vlans and add a member of a range of vlans
- How I did it
Added the following new commands:
create a range of vlans
delete a range o vlans
add a member of range of vlans
remove a member from a range of vlans

- How to verify it

- 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)
config vlan range add 2 50
config vlan member range add 2 50 Ethernet0
config vlan member range del 2 50 Ethernet0
config vlan range del 2 50

@msftclas
Copy link

msftclas commented Apr 24, 2020

CLA assistant check
All CLA requirements met.

@lgtm-com
Copy link

lgtm-com bot commented Apr 24, 2020

This pull request introduces 2 alerts when merging 706af06 into fc719ad - view on LGTM.com

new alerts:

  • 2 for Unused local variable

config/main.py Outdated
warning_vlans_list.append(vid)
continue

pipe.hmset('VLAN|{}'.format(vlan), {'vlanid': vid})
Copy link
Contributor

@lguohan lguohan May 6, 2020

Choose a reason for hiding this comment

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

need use swsssdk as abstract layer of API. it is likely that current swsssdk does not provide such functionalities needed. but all db access need to go through the swss-common or swsssdk (plan to deprecate and replaced by swss-common). Bypassing them and calling directly using redis API makes us impossible to abstract the DB access layer and prevent us from improving db access in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lguohan if I understood correctly, should I add a new API in swss-common/swsssdk and call the API from here instead of using redis client API directly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Anil - he's asking you to look in https://github.com/Azure/sonic-swss-common for a suitable DB access function and use this. If you don't find one that meets your needs then please add it - thanks.

@lgtm-com
Copy link

lgtm-com bot commented May 8, 2020

This pull request introduces 2 alerts when merging e08f1bc into 4a47b3f - view on LGTM.com

new alerts:

  • 2 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented Jun 25, 2020

This pull request introduces 3 alerts when merging f9c6a36 into 9950955 - view on LGTM.com

new alerts:

  • 2 for Unused local variable
  • 1 for Unused import

@anilkpan
Copy link
Contributor Author

Added new APIs for bulk update:
sonic-net/sonic-py-swsssdk#82

@lgtm-com
Copy link

lgtm-com bot commented Jun 25, 2020

This pull request introduces 1 alert when merging 8f81ae4 into 9950955 - view on LGTM.com

new alerts:

  • 1 for Syntax error

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.

5 participants