Splitting 'show' and 'clear' parse-chains to address overlapping functionality#128
Splitting 'show' and 'clear' parse-chains to address overlapping functionality#128jleveque merged 7 commits intosonic-net:masterfrom
Conversation
setup.py
Outdated
There was a problem hiding this comment.
I believe you also need to add entry points here under console_scripts for debug and undebug
There was a problem hiding this comment.
Right, will add those two right away.
MANIFEST.in
Outdated
There was a problem hiding this comment.
Why is this line needed? I believe we should be able to specify all needed .py files via setup.py, either via packages or scripts.
There was a problem hiding this comment.
That's part of a long story. I had a very hard time trying to add the new bgp_*.py files into a show/bgp/ and clear/bgp subfolders. I gave up at the end and i left my new files at the same level as main.py. This added line, reflects part of my effort tweaking packages' building-config to add the new bgp folder to the .deb package. Even though i was able to incorporate the new folders into the generated python-sonic-util.tar.gz, these ones never made it into the python-sonic-util.deb package. I gave up at the end, and i decided to remove the bgp folders, so I will go ahead and remove this line now.
There was a problem hiding this comment.
I recommend you please upload or paste these files to http://pep8online.com/ and make changes accordingly as it will help us keep our Python code style consistent.
I don't normally bother shortening lines that are a handful of characters too long, but I normally correct all other issues.
show/bgp_frr_v4.py
Outdated
There was a problem hiding this comment.
this is not supported, because we did not enable route cache in quagga. use show bgp ipv4 neighbor {} routes instead.
There was a problem hiding this comment.
Good catch. Will remove this command for the time being.
show/main.py
Outdated
There was a problem hiding this comment.
do we have this dmidecode in the base image? also, which dmidecode is related to hardware. This is not requirement for platform support, there is no guarantee that this will be supported by all sonic platforms.
There was a problem hiding this comment.
No, it's not in base-image, i was planning to add it though. However, i just noticed that you guys recently introduced "show platform", which overlaps with the info provided by this command, so i will remove this one from my patch.
|
I have taken care of all the comments posted above. Please let me know if there's anything else missing. |
clear/main.py
Outdated
There was a problem hiding this comment.
THis -> This (lowercase 'h').
clear/bgp_frr_v4.py
Outdated
There was a problem hiding this comment.
Many of these commands and groups are missing docstrings. The Click library uses the docstrings to present usage information to the user as part of the 'help' text.
Could you please go through all files and add all missing docstrings; just a brief description of each group/command?
Test each group/command by using the --help flag.
|
code looks good to me. |
|
resolve the conflicts before merging? |
|
Still conflicted? |
|
i used github's 'resolve-conflicts' tab directly to fix the conflict (see '359c1fc' above), but not sure we have enough 'rights' to edit the code through this channel. Are you still seeing conflicts? |
|
Yes. Still seeing conflicts here. I've never used the Github interface for resolving conflicts, so I'm not sure what might have gone wrong. |
…tionality. The following patch provides a simple mechanism to allow overlapping parse-chains to co-exist without interfering with each other. In this particular example, we are separating BGP functionality attending to the routing-stack being used in the system. As part of these changes, Quagga-based systems will exclusively rely on 'show ip/ipv6 bgp' and 'clear ip/ipv6 bgp' stanzas, whereas FRR will make use of 'show bgp ipv4/ipv6' and 'clear bgp ipv4/ipv6' ones. Also, a few bugs are getting fixed here. RB= G=lnos-reviewers R=ntrianta,rjonnadu,rmolina,sfardeen,zxu A=
Align style with slightly modified PEP8 standards (extend maximum line length to 120 chars). This will also help in the transition to Python 3, where it is more strict about whitespace. Done using `autopep8 --in-place --max-line-length 120` and some manual tweaks.
following PR's : Implement cbgpPeer2State in CiscoBgp4MIB (sonic-net#119) Fix index nodes in LLDP tables whose access right is not-accessible. (sonic-net#112) Fix quagga/FRR parser on IPv6 BGP sessions (sonic-net#122) [lint] Fix some syntax errors or warnings (sonic-net#127) Update README.md: Add lgtm badges (sonic-net#128) [Multi-asic]: Support multi-asic platform (sonic-net#126) Simplify test code (sonic-net#132) [Multi-asic]: Namespace support for LLDP and Sensor tables (sonic-net#131) Fix undefined variable and warning message (sonic-net#134) Fix SNMP AgentX socket connection timeout when using Namespace.get_all() (sonic-net#140) [Namespace] Fix interfaces counters in InterfacesMIB RFC 2863 (sonic-net#141) Fix LGTM reported alert of PR#141 (sonic-net#142)
…'s eeprom and configuring Y cable's mux through eeprom (sonic-net#128) * [SONIC_PLATFORM_COMMON] Adding new Y cable Package Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
…-net#128) <!-- Please make sure you've read and understood our contributing guidelines: https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md failure_prs.log skip_prs.log Make sure all your commits include a signature generated with `git commit -s` ** If this is a bug fix, make sure your description includes "closes #xxxx", "fixes #xxxx" or "resolves #xxxx" so that GitHub automatically closes the related issue when the PR is merged. If you are adding/modifying/removing any command or utility script, please also make sure to add/modify/remove any unit tests from the tests directory as appropriate. If you are modifying or removing an existing 'show', 'config' or 'sonic-clear' subcommand, or you are adding a new subcommand, please make sure you also update the Command Line Reference Guide (doc/Command-Reference.md) to reflect your changes. Please provide the following information: --> #### What I did CLI support for WRED and ECN statistics feature #### How I did it * New script for wredstat CLI commands * portstat script updated to accomodate WRED port stats * counterpoll script updated to support wredport and wredqueue counters * CLi to script mapping changes * UT for the new script changes #### How to verify it sonic-utilities UT and Marvell DUT manual testing #### Previous command output (if the output of a command-line utility has changed) root@sonic-dut:~# show interfaces counters detailed Ethernet8 Packets Received 64 Octets..................... 0 Packets Received 65-127 Octets................. 2 Packets Received 128-255 Octets................ 0 Packets Received 256-511 Octets................ 0 Packets Received 512-1023 Octets............... 0 Packets Received 1024-1518 Octets.............. 0 Packets Received 1519-2047 Octets.............. 0 Packets Received 2048-4095 Octets.............. 0 Packets Received 4096-9216 Octets.............. 0 Packets Received 9217-16383 Octets............. 0 Total Packets Received Without Errors.......... 2 Unicast Packets Received....................... 0 Multicast Packets Received..................... 2 Broadcast Packets Received..................... 0 Jabbers Received............................... N/A Fragments Received............................. N/A Undersize Received............................. 0 Overruns Received.............................. 0 Packets Transmitted 64 Octets.................. 32,893 Packets Transmitted 65-127 Octets.............. 16,449 Packets Transmitted 128-255 Octets............. 3 Packets Transmitted 256-511 Octets............. 2,387 Packets Transmitted 512-1023 Octets............ 0 Packets Transmitted 1024-1518 Octets........... 0 Packets Transmitted 1519-2047 Octets........... 0 Packets Transmitted 2048-4095 Octets........... 0 Packets Transmitted 4096-9216 Octets........... 0 Packets Transmitted 9217-16383 Octets.......... 0 Total Packets Transmitted Successfully......... 51,732 Unicast Packets Transmitted.................... 0 Multicast Packets Transmitted.................. 18,840 Broadcast Packets Transmitted.................. 32,892 Time Since Counters Last Cleared............... None #### New command output (if the output of a command-line utility has changed) root@sonic-dut:~# show interfaces counters detailed Ethernet8 Packets Received 64 Octets..................... 0 Packets Received 65-127 Octets................. 2 Packets Received 128-255 Octets................ 0 Packets Received 256-511 Octets................ 0 Packets Received 512-1023 Octets............... 0 Packets Received 1024-1518 Octets.............. 0 Packets Received 1519-2047 Octets.............. 0 Packets Received 2048-4095 Octets.............. 0 Packets Received 4096-9216 Octets.............. 0 Packets Received 9217-16383 Octets............. 0 Total Packets Received Without Errors.......... 2 Unicast Packets Received....................... 0 Multicast Packets Received..................... 2 Broadcast Packets Received..................... 0 Jabbers Received............................... N/A Fragments Received............................. N/A Undersize Received............................. 0 Overruns Received.............................. 0 Packets Transmitted 64 Octets.................. 32,893 Packets Transmitted 65-127 Octets.............. 16,449 Packets Transmitted 128-255 Octets............. 3 Packets Transmitted 256-511 Octets............. 2,387 Packets Transmitted 512-1023 Octets............ 0 Packets Transmitted 1024-1518 Octets........... 0 Packets Transmitted 1519-2047 Octets........... 0 Packets Transmitted 2048-4095 Octets........... 0 Packets Transmitted 4096-9216 Octets........... 0 Packets Transmitted 9217-16383 Octets.......... 0 Total Packets Transmitted Successfully......... 51,732 Unicast Packets Transmitted.................... 0 Multicast Packets Transmitted.................. 18,840 Broadcast Packets Transmitted.................. 32,892 Time Since Counters Last Cleared............... None WRED Green Dropped Packets..................... 1 WRED Yellow Dropped Packets.................... 3 WRED RED Dropped Packets....................... 10 WRED Total Dropped Packets..................... 14
The following patch provides a simple mechanism to allow overlapping parse-chains to co-exist without interfering with each other. In this particular example, we are separating BGP functionality attending to the routing-stack being used in the system. As part of these changes, Quagga-based systems will exclusively rely on 'show ip/ipv6 bgp' and 'sonic-clear ip/ipv6 bgp' stanzas, whereas FRR will make use of 'show bgp ipv4/ipv6' and 'sonic-clear bgp ipv4/ipv6' ones. Also, a few bugs are getting fixed here.