Enhancement of 'show' commands, addition of 'Clear', 'debug', 'undebug'. #98

samaity · 2017-08-18T04:19:33Z

List of changes:

    > added script to get interface status.
    > added few subcommands under "show interface" command.
    > enhanced "show process" to get all processes sorted by CPU & memory.
    > added "show services" command to get all the running process from all the dockers.
    > added "show vlan" command.
    > enhanced multiple subcommands under 'show ip bgp' command.
    > added 'show ipv6 bgp' command and multiple subcommands under it.
    > added 'Clear', 'debug', and 'undebug' CLI utilities.

jleveque · 2017-08-18T18:36:42Z

scripts/aclshow

@@ -229,7 +229,7 @@ class AclStat(object):
            if not display_all and (self.get_counter_value(rule_key, 'packets') == '0' or \
                    self.get_counter_value(rule_key, 'packets') == 'N/A'):
                continue
-            rule = self.acl_rules[rule_key]
+	    rule = self.acl_rules[rule_key]


This appears to be an accidental change. Revert?

jleveque · 2017-08-18T19:14:24Z

Clear/main.py

+from click_default_group import DefaultGroup
+
+try:
+    # noinspection PyPep8Naming


These # noinspection markups are specific to the PyCharm IDE. Please remove them.

jleveque · 2017-08-18T19:14:45Z

Clear/main.py

+    # noinspection PyPep8Naming
+    import ConfigParser as configparser
+except ImportError:
+    # noinspection PyUnresolvedReferences


Remove markup, as above.

jleveque · 2017-08-18T19:15:13Z

debug/main.py

+from click_default_group import DefaultGroup
+
+try:
+    # noinspection PyPep8Naming


Remove markup, as above.

jleveque · 2017-08-18T19:15:18Z

debug/main.py

+    # noinspection PyPep8Naming
+    import ConfigParser as configparser
+except ImportError:
+    # noinspection PyUnresolvedReferences


Remove markup, as above.

jleveque · 2017-08-18T19:15:29Z

undebug/main.py

+    # noinspection PyPep8Naming
+    import ConfigParser as configparser
+except ImportError:
+    # noinspection PyUnresolvedReferences


Remove markup, as above.

jleveque · 2017-08-18T19:15:35Z

undebug/main.py

+from click_default_group import DefaultGroup
+
+try:
+    # noinspection PyPep8Naming


Remove markup, as above.

jleveque · 2017-08-18T19:19:43Z

Clear/main.py

@@ -0,0 +1,337 @@
+#! /usr/bin/python -u


I understand that you've named this program Clear with a capital "C" to distinguish it from the standard Linux clear command. I feel as though it would be better off with a different, all-lowercase name to avoid confusion. Suggestions?

Alternative is to alias "clear" with some script. In that script, if clear does not have arguments, invoke system clear command, otherwise invoke corresponding sonic clear command.

maybe "clearcmd" ?

The best way to address this is by having a small script with name "clear" installed under /usr/local/bin/ that redirects user input to system clear command (/usr/bin/clear) & SONiC Clear commands (/usr/bin/Clear).
The user will not see the upper case Clear & the redirection will transparent to the user.
Since, that script change has go to sonic-buildimage repo, I can commit that change in a separate pull.

can we rename /usr/bin/clear to /usr/bin/clear_screen

then, when we do 'clear screen', we invoke this /usr/bin/clear_screen

This isn't a backwards compatible suggestion if we ever decide to change in the future the cli framework. The Linux cmds shouldn't be touched.

if there is no argument, we can still call linux "clear", there is no backwards compatibility issue.

/usr/bin/clear is the linux clear screen command. It should not be replaced and it should not be renamed. There needs to be a sonic-clear and a clear in /usr/local/bin. In the user's path /usr/local/bin comes first. The clear in /usr/local/bin with no arguments should call /usr/bin/clear and with arguments it should call /usr/local/bin/sonic-clear.

The only issue I have with Ravi's solution of a /usr/local/bin/clear script is that it will make the SONiC clear command behave inconsistently with the other CLI commands. For instance, calling show or config with no arguments will present the user with the usage statement, but calling clear with no arguments will clear the user's screen.

It is unfortunate that sonic presents the user with usage statement when show or config are called with no arguments. Sounds like that decision needs to be revisited. Nevertheless, we need to move forward with this commit. I agree with the point you made that we should come up with a better way than a capitalized command. Sort of introducing a cli shell for all sonic commands (show, clear, config), this is the next best thing which is not interfering with the linux clear command in any way.

stcheng · 2017-08-25T18:41:53Z

Clear/main.py

+
+def run_command(command, pager=False):
+    if pager is True:
+        #click.echo(click.style("Command: ", fg='cyan') + click.style(command, fg='green'))


why these two lines are commented out?

You mean the last line?

He's referring to both of the click.echo(click.style("Command: ", fg='cyan') + click.style(command, fg='green')) lines (lines 86 and 90). These are carried over from our show command, in which they display the underlying wrapped command that is being called. In our config command, I added a "verbose" flag to enable this print, leaving it disabled by default. I have not yet got around to modifying show to match. I believe this utility should either follow the same format or these lines should be deleted altogether.

stcheng · 2017-08-25T18:42:50Z

Clear/main.py

+# 'cli' group (root group) ###
+#
+
+# THis is our entrypoint - the main "Clear" command


Looks like this was missed: THis -> This.

This was a copy-paste of a typo I made in the show command. I have since fixed it there.

jleveque · 2017-09-22T00:57:04Z

Clear/main.py

+
+# We use the "click.group()" decorator because we want to add this group
+# to more than one group, which we do using the "add_command() methods below.
+@click.group(cls=AliasedGroup,default_if_no_args=True)


I have made changes to the group/command hierarchy in the show command, so this should be updated to match. My changes are here for your reference: #102

The bgp group should now only be a member of the ip group, NOT the cli group. Thus, line 125 should change to @ip.group(cls=AliasedGroup, default_if_no_args=False) and you can subsequently remove lines 131-133.

The change made in #102 regarding BGP is not necessarily the right approach. BGP has many address families (l2vpn, ls etc) and they are certainly not ipv4 or ipv6. BGP should either have "show ip bgp ..." or "show bgp ..." or both. BGP should not appear under ipv6.

jleveque · 2017-09-22T00:58:32Z

Clear/main.py

+
+
+# 'bgp' command
+@click.group(cls=AliasedGroup,default_if_no_args=True)


Same as above. This bgp command is only a member of the ipv6 group, so you can change line 224 to @ipv6.group(cls=AliasedGroup,default_if_no_args=True) and subsequently remove line 229.

See comment above.

jleveque · 2017-09-22T01:01:39Z

Clear/main.py

+def neighbor():
+    pass
+
+bgp.add_command(neighbor)


This line is unnecessary, as neighbor is added to the bgp group via the decorator on line 239.

jleveque · 2017-09-22T01:02:21Z

Clear/main.py

+def soft():
+    pass
+
+neighbor.add_command(soft)


This line is unnecessary, as neighbor is added to the neighbor group via the decorator on line 179.

jleveque · 2017-09-22T01:02:50Z

Clear/main.py

+def soft():
+    pass
+
+neighbor.add_command(soft)


This line is unnecessary, as neighbor is added to the neighbor group via the decorator on line 275.

jleveque · 2017-09-22T01:06:44Z

Clear/main.py

+# 'arp' command ####
+#
+
+@click.command()


For now, only add arp command to the root cli group, so you can change line 320 to @cli.command() and subsequently remove lines 330-332.

stcheng · 2017-09-25T20:55:37Z

Please also address all the conflicts.

jleveque · 2017-09-27T23:58:30Z

show/main.py

+	if e.errno == errno.EPIPE:
+		sys.exit(0)
+	else:
+		raise


Revert back to spaces from tabs (lines 98-102).

jleveque · 2017-09-27T23:58:50Z

show/main.py

    if interfacename is not None:
-        cmd += " -p {}".format(interfacename)
+	cmd += " -p {}".format(interfacename)


Revert back to spaces from tabs.

jleveque · 2017-09-27T23:59:42Z

show/main.py

+@click.argument('interfacename', required=True)
+def details(interfacename):
+	command="sudo sfputil --port {} --dom".format(interfacename)
+	run_command(command)


Replace spaces with tabs (lines 184-185).

jleveque · 2017-09-28T00:00:36Z

show/main.py

+	t = p1.stdout.readlines()[0]
+	command2 = "cat {}/port_config.ini".format(t.rstrip())
+	p2 = subprocess.Popen(command2, shell=True, stdout=subprocess.PIPE)
+	click.echo(p2.stdout.read())


Replace spaces with tabs (lines 242-259).

jleveque · 2017-09-28T00:01:54Z

show/main.py

+@neighbor.command('advertised-routes')
+@click.argument('ipaddress')
+def advertised_routes(ipaddress):
+	command = 'sudo vtysh -c "show ip bgp neighbor {} advertised-routes"'.format(ipaddress)


Replace spaces with tabs.

jleveque · 2017-09-28T00:06:10Z

show/main.py

+@cli.command('system-memory')
+def system_memory():
+    """Show memory information"""
+    comm="free -m"


Change to command = "..." for consistency ("comm" -> "command" and add spaces around equals sign).

jleveque · 2017-09-28T00:06:16Z

show/main.py

+@click.argument('bridge_name', required=True)
+def id(bridge_name):
+    """Show list of learned MAC addresses for particular bridge"""
+    comm="sudo brctl showmacs {}".format(bridge_name)


Change to command = "..." for consistency ("comm" -> "command" and add spaces around equals sign).

jleveque · 2017-09-28T00:06:42Z

show/main.py

+@cli.command('services')
+def services():
+    """Show all daemon services"""
+    comm = "sudo docker ps --format '{{.Names}}'"


Change to command = "..." for consistency ("comm" -> "command").

jleveque · 2017-09-28T00:09:37Z

show/main.py

+        if line != '':
+                print(line.rstrip()+'\t'+"docker")
+                print("---------------------------")
+                comm1 = "sudo docker exec -it {} ps -ef | sed '$d'".format(line.rstrip())


Reuse command instead of creating comm1?

jleveque · 2017-09-28T00:11:27Z

show/main.py

+    run_command("interface_stat")
+
+@interface.command()
+def portmap():


Remove portmap command as no it is redundant with show interfaces alias.

Fix `Failed to update PSU data - global name 'self' is not defined` Signed-off-by: Volodymyr Boyko <volodymyrx.boiko@intel.com>

[sfp_base] Update return value documentation of channel-specific methods (sonic-net#98) [SfpBase] Fix key name typo in docstring (sonic-net#99) [sfp] Tweak key names of some transceiver info fields (sonic-net#97) [sfputil] Make SfpUtilHelper.get_physical_to_logical noexcept as in SfpUtilBase (sonic-net#96)

a1830c1761087bdc1f7433ebbb8d0bdc419da0d3 (HEAD -> 201911, origin/master, origin/HEAD, origin/201911) Fix OpenAPI spec to be readable by autorest (sonic-net#101) 94805a39ac0712219f7dc08faa2cfdbf371dd177 Identify and report Vnet GUID for conflicting VNI (sonic-net#99) 4832dfd677de72edc44d4eb8c1b60cfad79a3355 Static route expiry if not specified as persistent (sonic-net#98) 5cc4358fb67b9e2a0da9a6691064e41f97ebebc2 (master) Add support for overlay ECMP (sonic-net#96) 6822a46197daef060b4d00dba5153b04b163c43f [CI] Set diff cover threshold to 50% (sonic-net#97) dcc826a1503060b9a07e4510b4f48331c49e87dd Add PR diff coverage (sonic-net#95) e842c5ff317c67919dcbcab3358143cb9a16c9dd Generate code coverage for Unit Tests (sonic-net#94) f9bbed3cb86a3bab9a07745096835dbdbe5a4db6 Convert Unit Tests from unittest framework to pytest framework (sonic-net#93) Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

…ods (sonic-net#98) Update docstrings of `get_rx_los()`, `get_tx_fault()` and `get_tx_disable()` methods to reflect that they should return a list of values where each value reflect the status of an individual SFP channel. Also add more detail to other channel-specific method docstrings.

msftclas added cla-required and removed cla-required labels Aug 18, 2017

jleveque reviewed Aug 18, 2017

View reviewed changes

jleveque suggested changes Aug 18, 2017

View reviewed changes

jleveque reviewed Aug 18, 2017

View reviewed changes

stcheng reviewed Aug 25, 2017

View reviewed changes

jleveque suggested changes Sep 22, 2017

View reviewed changes

msftgits removed the cla-signed label Sep 26, 2017

sonic-net deleted a comment from msftclas Sep 26, 2017

nikos-github mentioned this pull request Sep 27, 2017

[show]: Add 'show interfaces alias' command to display port name/alias mapping #107

Merged

jleveque suggested changes Sep 28, 2017

View reviewed changes

samaity force-pushed the master branch 2 times, most recently from aaa1ce6 to 150c8a6 Compare September 29, 2017 18:17

samaity closed this Sep 29, 2017

samaity force-pushed the master branch from 150c8a6 to 206aabe Compare September 29, 2017 18:26

vdahiya12 pushed a commit to vdahiya12/sonic-utilities that referenced this pull request Jul 23, 2021

[psud] Fix psud logging (sonic-net#98)

8507085

Fix `Failed to update PSU data - global name 'self' is not defined` Signed-off-by: Volodymyr Boyko <volodymyrx.boiko@intel.com>



		# 'bgp' command
		@click.group(cls=AliasedGroup,default_if_no_args=True)

Enhancement of 'show' commands, addition of 'Clear', 'debug', 'undebug'. #98

Enhancement of 'show' commands, addition of 'Clear', 'debug', 'undebug'. #98

Conversation

samaity commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan Sep 22, 2017 • edited Loading

Choose a reason for hiding this comment

nikos-github Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikos-github Sep 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikos-github Sep 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jleveque Sep 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikos-github Sep 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikos-github Sep 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stcheng commented Sep 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan Sep 22, 2017 •

edited

Loading

nikos-github Sep 25, 2017 •

edited

Loading

nikos-github Sep 26, 2017 •

edited

Loading

nikos-github Sep 26, 2017 •

edited

Loading

jleveque Sep 15, 2017 •

edited

Loading

nikos-github Sep 26, 2017 •

edited

Loading

nikos-github Sep 26, 2017 •

edited

Loading