Address review comments

- Use click library as suggested by Wenyi to simplify argument parsing
- Support --namespace argument conditionally. If not passed, the default
  behavior is to display all namespaces.
- Utilize `run_on_multi_asic` decorator to abstract iterating over
  namespaces
- Added and updating UTs

scripts/dropstat

@@ -11,8 +11,8 @@
 # - Refactor calls to COUNTERS_DB to reduce redundancy
 # - Cache DB queries to reduce # of expensive queries
 
+import click
 import json
-import argparse
 import os
 import socket
 import sys
@@ -22,6 +22,7 @@ from natsort import natsorted
 from tabulate import tabulate
 from sonic_py_common import multi_asic
 from utilities_common.general import load_db_config
+import utilities_common.multi_asic as multi_asic_util
 
 # mock the redis for unit test purposes #
 try:
@@ -96,14 +97,11 @@ def get_dropstat_dir():
 
 class DropStat(object):
     def __init__(self, namespace):
-        self.config_db = ConfigDBConnector(namespace=namespace, use_unix_socket_path=True if namespace else False)
-        self.config_db.connect()
-
-        self.db = SonicV2Connector(namespace=namespace, use_unix_socket_path=True if namespace else False)
-        self.db.connect(self.db.COUNTERS_DB)
-        self.db.connect(self.db.ASIC_DB)
-        self.db.connect(self.db.APPL_DB)
-        self.db.connect(self.db.CONFIG_DB)
+        self.namespaces = multi_asic.get_namespace_list(namespace)
+        self.multi_asic = multi_asic_util.MultiAsic(namespace_option=namespace)
+        self.db = None
+        self.config_db = None
+        self.cached_namespace = None
 
         dropstat_dir = get_dropstat_dir()
         self.port_drop_stats_file = os.path.join(dropstat_dir, 'port-stats')
@@ -113,6 +111,7 @@ class DropStat(object):
         self.stat_lookup = {}
         self.reverse_stat_lookup = {}
 
+    @multi_asic_util.run_on_multi_asic
     def show_drop_counts(self, group, counter_type):
         """
             Prints out the current drop counts at the port-level and
@@ -326,12 +325,13 @@ class DropStat(object):
             the given object type.
         """
 
+        if self.cached_namespace != self.multi_asic.current_namespace:
+            self.stat_lookup = {}
+            self.cached_namespace = self.multi_asic.current_namespace
+
         if not self.stat_lookup.get(object_stat_map, None):
             stats_map = self.db.get_all(self.db.COUNTERS_DB, object_stat_map)
-            if stats_map:
-                self.stat_lookup[object_stat_map] = stats_map
-            else:
-                self.stat_lookup[object_stat_map] = None
+            self.stat_lookup[object_stat_map] = stats_map if stats_map else None
 
         return self.stat_lookup[object_stat_map]
 
@@ -462,49 +462,22 @@ class DropStat(object):
         else:
             return PORT_STATE_NA
 
-
-def main():
-    parser = argparse.ArgumentParser(description='Display drop counters',
-                                     formatter_class=argparse.RawTextHelpFormatter,
-                                     epilog="""
-Examples:
-  dropstat
-""")
-
-    # Version
-    parser.add_argument('-v', '--version', action='version', version='%(prog)s 1.0')
-
-    # Actions
-    parser.add_argument('-c', '--command', type=str, help='Desired action to perform')
-
-    # Variables
-    parser.add_argument('-g', '--group',   type=str, help='The group of the target drop counter', default=None)
-    parser.add_argument('-t', '--type',    type=str, help='The type of the target drop counter', default=None)
-    parser.add_argument('-n', '--namespace', type=str, help='Namespace name', default=None)
-
-    args = parser.parse_args()
-
-    command = args.command
-
-    group = args.group
-    counter_type = args.type
-    namespace = args.namespace
-
+@click.command(help='Display drop counters')
+@click.option('-c', '--command', required=True, help='Desired action to perform',
+              type=click.Choice(['clear', 'show'], case_sensitive=False))
+@click.option('-g', '--group', default=None, help='The group of the target drop counter')
+@click.option('-t', '--type', 'counter_type', default=None, help='The type of the target drop counter')
+@click.option('-n', '--namespace', help='Namespace name', default=None,
+              type=click.Choice(multi_asic.get_namespace_list()))
+@click.version_option(version='1.0')
+def main(command, group, counter_type, namespace):
     load_db_config()
-    namespaces = multi_asic.get_namespace_list()
-    if namespace and namespace not in namespaces:
-        raise Exception("Input arguments error. Namespaces must be one of", *namespaces)
-
-    if multi_asic.is_multi_asic() and not namespace:
-        raise Exception("Input arguments error. Namespace must be specified for multi-asic devices.")
 
     dcstat = DropStat(namespace)
     if command == 'clear':
         dcstat.clear_drop_counts()
-    elif command == 'show':
-        dcstat.show_drop_counts(group, counter_type)
     else:
-        print("Command not recognized")
+        dcstat.show_drop_counts(group, counter_type)
 
 
 if __name__ == '__main__':

tests/multi_asic_dropstat_test.py

@@ -1,7 +1,7 @@
 import os
 import sys
 import shutil
-from utils import get_result_and_return_code
+from .utils import get_result_and_return_code
 
 test_path = os.path.dirname(os.path.abspath(__file__))
 modules_path = os.path.dirname(test_path)
@@ -14,8 +14,8 @@
 dropstat_masic_result = """\
        IFACE    STATE    RX_ERR    RX_DROPS    TX_ERR    TX_DROPS    DEBUG_0    DEBUG_2
 ------------  -------  --------  ----------  --------  ----------  ---------  ---------
-   Ethernet0        U         0           0         0           0          0          0
-   Ethernet4        U         0           0         0           0          0          0
+   Ethernet0        U        10         100         0           0         80         20
+   Ethernet4        U         0        1000         0           0        800        100
 Ethernet-BP0        U         0        1000         0           0        800        100
 Ethernet-BP4        U         0        1000         0           0        800        100
 
@@ -50,7 +50,8 @@ def test_show_pg_drop_masic_invalid_ns(self):
         ])
         print("return_code: {}".format(return_code))
         print("result = {}".format(result))
-        assert return_code == 1
+        assert return_code == 2
+        assert "asic5' is not one of" in result
 
     @classmethod
     def teardown_class(cls):