[dvs_acl] Refactor and document dvs_acl library #1378
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request fixes 6 alerts when merging d446794 into ea30f2f - view on LGTM.com fixed alerts:
|
|
This pull request fixes 6 alerts when merging 9956169 into ea30f2f - view on LGTM.com fixed alerts:
|
|
retest vs please |
1 similar comment
|
retest vs please |
- Add docstrings to dvs_acl - Revert dvs_acl to a vanilla fixture - Combine redundant dvs_acl methods - Refactor test_acl, test_port_dpb_acl, and test_mirror_port_span to use new interface Signed-off-by: Danny Allen <[email protected]>
|
This pull request fixes 6 alerts when merging 03d33e7 into 2fe99e0 - view on LGTM.com fixed alerts:
|
lguohan
approved these changes
Aug 6, 2020
|
test failures are not related to this change |
|
This pull request fixes 6 alerts when merging 14a9170 into 6da7036 - view on LGTM.com fixed alerts:
|
EdenGri
pushed a commit
to EdenGri/sonic-swss
that referenced
this pull request
Feb 28, 2022
…sonic-net#1378) - What I did Remove dhcp relay commands from sonic-utilities. dhcp-relay commands will come as a plugin with dhcp-relay docker installation. See sonic-net/SONiC#682 - How I did it Remove dhcp-relay commands from vlan. Make "show vlan brief" command table output extendable. - How to verify it Install dhcp-relay docker as app.ext. Verify that "config vlan dhcp-relay" and "show vlan brief" show dhcp data.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Danny Allen [email protected]
What I did/why I did it
I changed
dvs_aclback into an explicit fixture that is passed in to the test methods. The reason for this is because I figured out that tools likepylintandflake8do not handle therequest.cls...update very well, which is going to be problematic very soon. In addition, this change makes it a little easier for new contributers/newdvslibadopters to see what is going on by making the inclusion of thedvs_aclmodule a little more explicit.I merged a few methods together in
dvs_aclthat were checking the same things (primarily the ACL group and port binding checks). This is for maintainability and to keep the public interface cleaner.I added a bunch of type hints and docstrings to
dvs_aclto make the usage more clear.I created some fixtures in
test_aclso that each individual test case can be run standalone, if the user asks for it. This should also make the PR tests more stable as it will be easier forflakyto re-run test cases in a controlled manner. Fixes [testing] vs tests in TestAcl in test_acl.py have dependency on each other #1154I deleted some dead ACL code from
conftestas it wasn't being used bytest_aclortest_port_dpb_aclanymore with thedvslibadditions.How I verified it
Ran the test suite locally and it passes w/ the refactor.
Details if related
Once this refactor is merged we can begin converting the rest of
test_acl_*andtest_mirror_*to usedvslib.