Replace None comparisons with 'is/is not' operators. Updated API with docstrings. (#271)

Signed-off-by: Andriy Kokhan <[email protected]>

Author: andriy-kokhan

README.md

@@ -22,7 +22,7 @@ SAI Challenger has many applications. A partial list is below:
 * CI/CD and regression testing of virtual or physical DUTs
 * DUT performance testing using HW traffic generators
 * PHY (transceiver) device testing & qualification
-* Ubrella test harness for native SAI Challenger test cases as well as legacy SAI-PTF test cases, using a single-pane-of-glass to reduce testbed complexity.
+* Umbrella test harness for native SAI Challenger test cases as well as legacy SAI-PTF test cases, using a single-pane-of-glass to reduce testbed complexity.
 
 # Use-case scenarios
 SAI Challenger has many configuration options, resulting in numerous permutations of:

common/sai.py

@@ -9,19 +9,46 @@
 
 class CommandProcessor:
     """
-    Allow setup scaled configurations with referenced objects.
-    Contain reference object cache.
-    When objects are referenced they are regenerating missing keys/oids from object cache
+    Process SAI commands with object reference substitution.
+
+    This class enables setup of scaled configurations by maintaining a registry
+    of created objects and substituting object references (variables starting with '$')
+    with actual OIDs or keys during command processing.
+
+    Attributes:
+        objects_registry: Dictionary mapping object names to their metadata
+        sai: Reference to parent Sai instance
     """
 
     class SubstitutionError(RuntimeError):
-        ...
+        """Raised when object reference substitution fails."""
+        pass
 
     def __init__(self, sai: 'Sai'):
+        """
+        Initialize CommandProcessor.
+
+        Args:
+            sai: Parent Sai instance for API calls
+        """
         self.objects_registry = {}
         self.sai = sai
 
     def _substitute_from_object_registry(self, obj, *args, **kwargs):
+        """
+        Substitute a reference variable with its actual value from registry.
+
+        Args:
+            obj: Object reference (string starting with '$') or regular value
+            *args: Optional default value as positional argument
+            **kwargs: Optional default value as 'default' keyword argument
+
+        Returns:
+            Registry entry for the referenced object
+
+        Raises:
+            SubstitutionError: If object is not found in registry and no default provided
+        """
         if isinstance(obj, str) and obj.startswith('$'):
             store_name = obj[1:]
             try:
@@ -35,6 +62,19 @@ def _substitute_from_object_registry(self, obj, *args, **kwargs):
             raise self.SubstitutionError
 
     def substitute_command_from_object_registry(self, command):
+        """
+        Substitute all object references in a command with actual values.
+
+        Processes command dictionary and replaces:
+        - '$VARIABLE' references in keys with actual OIDs/keys
+        - '$VARIABLE' references in attributes with actual values
+
+        Args:
+            command: Dictionary containing command with possible references
+
+        Returns:
+            New command dictionary with all references substituted
+        """
         substituted_command = {}
         store_name = command.get("name")
 
@@ -86,7 +126,9 @@ def substitute_command_from_object_registry(self, command):
         return substituted_command
 
     def process_command(self, command):
-        '''
+        """
+        Process a single SAI command (create, set, get, or remove).
+
         Command examples:
             {
                 "name": "vip1"
@@ -105,7 +147,16 @@ def process_command(self, command):
                 "type" : "SAI_OBJECT_TYPE_DASH_ACL_GROUP",
                 "attributes" : [ "SAI_DASH_ACL_GROUP_ATTR_IP_ADDR_FAMILY", "SAI_IP_ADDR_FAMILY_IPV4" ]
             },
-        '''
+
+        Args:
+            command: Dictionary with 'name', 'op', 'type', and optional 'key'/'attributes'
+
+        Returns:
+            Created OID for 'create', attribute values for 'get', or operation status
+
+        Raises:
+            AssertionError: If command format is invalid or operation fails
+        """
         store_name = command.get("name")
         assert store_name, f"Invalid command {command}. Entry name is undefined"
 

common/sai_client/sai_thrift_client/sai_thrift_client.py

@@ -126,10 +126,10 @@ def get(self, obj, attrs, do_assert=True):
         return status, SaiData(result)
 
     def get_object_type(self, oid, default=None) -> SaiObjType:
-        if default != None:
+        if default is not None:
             return ThriftConverter.convert_to_sai_obj_type(default)
 
-        if oid != None:
+        if oid is not None:
             try:
                 obj_type = self.sai_type_map.get(oid, None)
                 if obj_type is None:

common/sai_client/sai_thrift_client/sai_thrift_utils.py

@@ -120,7 +120,7 @@ def convert_value_to_thrift(value, attr_name=None, value_type=None):
                            'macsecsak', 'macsecauthkey', 'macsecsalt' ]:
             if isinstance(value, str):
                 actual_value = getattr(sai_headers, value, None)
-                if actual_value != None:
+                if actual_value is not None:
                     return actual_value
             if value == '':
                 return 0
@@ -393,7 +393,7 @@ def object_id(oid):
         "16"       => 16
         "oid:0x10" => 16
         """
-        if oid == None or oid == 'null' or oid == 'SAI_NULL_OBJECT_ID':
+        if oid is None or oid == 'null' or oid == 'SAI_NULL_OBJECT_ID':
             return 0
         if isinstance(oid, str) and oid.startswith('oid:0x'):
             return int(oid[4:], 16)
@@ -412,7 +412,7 @@ def get_value_type_by_thrift_spec(thrift_spec):
         # FIXME: Sometimes, thrift_spec returns "None" for both "oid" and "int"
         #        E.g., For SAI_OBJECT_TYPE_DIRECTION_LOOKUP_ENTRY, thrift_spec will be
         #        (1, 10, 'switch_id', None, None), (2, 8, 'vni', None, None)
-        if thrift_spec == None:
+        if thrift_spec is None:
             return "oid"
 
         attribute_value_spec = getattr(ttypes, f'sai_thrift_attribute_value_t').thrift_spec
@@ -441,7 +441,7 @@ def convert_value_from_thrift(value, attr_name, obj_type=None):
             return str(value) if value > 0 else str(value & 0xFFFF)
         elif value_type in [ 's32' ]:
             actual_value = ThriftConverter.get_str_by_enum(obj_type, attr_name, value)
-            if actual_value != None:
+            if actual_value is not None:
                 return actual_value
             return str(value)
         elif value_type in [ 'booldata' ]:
@@ -497,7 +497,7 @@ def from_sai_int_list(value_type, object_list, attr_name=None, obj_type=None):
         for ii in range(object_list.count):
             if value_type == 's32list' and attr_name is not None:
                 actual_value = ThriftConverter.get_str_by_enum(obj_type, attr_name, listvar[ii])
-                if actual_value != None:
+                if actual_value is not None:
                     listvar[ii] = actual_value
             result += str(listvar[ii])
             result += ","
@@ -644,7 +644,7 @@ def get_str_by_enum(obj_type, attr_name, enum_value):
         meta = ThriftConverter.get_sai_meta(obj_type, attr_name)
         if meta is None:
             return None
-        if meta['properties'].get('values') == None:
+        if meta['properties'].get('values') is None:
             return str(enum_value)
         for k, v in meta['properties']['values'].items():
             if v == enum_value:

common/sai_data.py

@@ -3,9 +3,22 @@
 
 
 class SaiObjType(Enum):
+    """
+    Enumeration for SAI object types.
+
+    This enum can be dynamically populated from either Thrift headers
+    or SAI JSON metadata files.
+    """
 
     @staticmethod
     def generate_from_thrift():
+        """
+        Generate SAI object type enum from Thrift headers.
+
+        Dynamically populates the enum with SAI_OBJECT_TYPE_* constants
+        found in the sai_thrift.sai_headers module. Skips if enum is
+        already populated.
+        """
         # Skip generation in case enum is not empty
         if list(SaiObjType):
             return
@@ -35,6 +48,12 @@ def generate_from_thrift():
 
     @staticmethod
     def generate_from_json():
+        """
+        Generate SAI object type enum from JSON metadata.
+
+        Dynamically populates the enum with SAI object types from
+        /etc/sai/sai.json metadata file. Skips if enum is already populated.
+        """
         # Skip generation in case enum is not empty
         if list(SaiObjType):
             return
@@ -50,6 +69,12 @@ def generate_from_json():
 
 
 class SaiStatus(Enum):
+    """
+    Enumeration for SAI operation status codes.
+
+    Contains all standard SAI status codes including success, failures,
+    and attribute-specific error codes.
+    """
     SUCCESS                     =  0x00000000
     FAILURE                     = -0x00000001
     NOT_SUPPORTED               = -0x00000002
@@ -87,22 +112,39 @@ class SaiStatus(Enum):
 
 
 class SaiData:
+    """
+    Wrapper class for SAI data returned from API calls.
+
+    Provides convenient methods to parse and extract data in various
+    formats (JSON, OID, lists, counters, etc.).
+    """
+
     def __init__(self, data):
+        """
+        Initialize SaiData with raw data.
+
+        Args:
+            data: Raw data string from SAI API call
+        """
         self.data = data
 
     def raw(self):
+        """Return raw data string."""
         return self.data
 
     def to_json(self):
+        """Parse and return data as JSON."""
         return json.loads(self.data)
 
     def oid(self, idx=1):
+        """Extract OID from JSON data at the specified index."""
         value = self.to_json()[idx]
         if "oid:" in value:
             return value
         return "oid:0x0"
 
     def to_list(self, idx=1):
+        """Convert data at index to a list by parsing format 'count:item1,item2,...'."""
         value = self.to_json()[idx]
         n_items, _, items = value.partition(':')
         if n_items.isdigit():
@@ -111,13 +153,15 @@ def to_list(self, idx=1):
         return []
 
     def oids(self, idx=1):
+        """Extract list of OIDs from data at the specified index."""
         value = self.to_list(idx)
         if len(value) > 0:
             if "oid:" in value[0]:
                 return value
         return []
 
     def counters(self):
+        """Parse statistics counters into a dictionary."""
         i = 0
         cntrs_dict = {}
         value = self.to_json()
@@ -127,12 +171,14 @@ def counters(self):
         return cntrs_dict
 
     def value(self):
+        """Extract the value from JSON data, normalizing boolean values."""
         if self.to_json()[1].lower() in ["true", "false"]:
             # Thrift to Redis compatibility
             return self.to_json()[1].lower()
         return self.to_json()[1]
 
     def uint32(self):
+        """Extract and return value as unsigned 32-bit integer."""
         v = self.value()
         assert v.isdigit(), f"Unexpected {self.to_json()[0]} value {v} "
         return int(v)

common/sai_dataplane/ptf/sai_ptf_dataplane.py

@@ -94,7 +94,7 @@ def __init__(self, cfg=None):
     def setUp(self):
         assert self.dataplane is not None
         self.dataplane.flush()
-        if config["log_dir"] != None:
+        if config["log_dir"] is not None:
             filename = os.path.join(config["log_dir"], str(self)) + ".pcap"
             self.dataplane.start_pcap(filename)
 
@@ -106,7 +106,7 @@ def at_receive(self, pkt, device_number=0, port_number=-1):
 
     def tearDown(self):
         assert self.dataplane is not None
-        if config["log_dir"] != None:
+        if config["log_dir"] is not None:
             self.dataplane.stop_pcap()
 
     @staticmethod
@@ -135,7 +135,7 @@ def __logging_setup(config):
 
         logging.getLogger().setLevel(DEBUG_LEVELS[config["debug"]])
 
-        if config["log_dir"] != None:
+        if config["log_dir"] is not None:
             if os.path.exists(config["log_dir"]):
                 import shutil
                 shutil.rmtree(config["log_dir"])
@@ -234,7 +234,7 @@ def init(self):
 
         # Set up the dataplane
         dataplane_instance = ptf.dataplane.DataPlane(config)
-        if config["log_dir"] == None:
+        if config["log_dir"] is None:
             filename = os.path.splitext(config["log_file"])[0] + '.pcap'
             dataplane_instance.start_pcap(filename)
 

common/sai_dataplane/sai_hostif_dataplane.py

@@ -17,7 +17,7 @@ def init(self):
         _config = copy.deepcopy(config)
         _config["platform"] = "nn"
         self.dataplane = DataPlane(_config)
-        if _config["log_dir"] == None:
+        if _config["log_dir"] is None:
             filename = os.path.splitext(_config["log_file"])[0] + '.pcap'
             self.dataplane.start_pcap(filename)
 

common/sai_dataplane/snappi/sai_snappi_dataplane.py

@@ -253,7 +253,7 @@ def add_ethernet_header(self,
                             src_count=1,
                             src_step="00:00:00:00:00:01"
                             ):
-        if flow == None:
+        if flow is None:
             return None
 
         ether = flow.packet.add().ethernet
@@ -285,7 +285,7 @@ def add_ipv4_header(self,
                         src_count=1,
                         src_step="0.0.0.1"
                         ):
-        if flow == None:
+        if flow is None:
             return None
 
         ipv4 = flow.packet.add().ipv4
@@ -309,7 +309,7 @@ def add_udp_header(self,
                         src_count=1,
                         src_step=1
                         ):
-        if flow == None:
+        if flow is None:
             return None
 
         udp = flow.packet.add().udp
@@ -329,7 +329,7 @@ def add_vxlan_header(self,
                         vni_count=1,
                         vni_step=1
                         ):
-        if flow == None:
+        if flow is None:
             return None
 
         vxlan = flow.packet.add().vxlan