Improve code quality by reducing complexity and fixing SonarQube issues across bessctl and related modules

bessctl/conf/port/vhost/launch_vm.py

@@ -40,7 +40,7 @@
 import time
 import shlex
 
-from qmp import QEMUMonitorProtocol
+from qmp import QEMUMonitorProtocol, QMPError
 
 # How many cores we reserve for vSwitches?
 # If set to 2, VMs will run on core 2, 3, 4, ..., skipping core 0-1.
@@ -91,7 +91,7 @@ def get_threads(path):
     def do_command(srv, cmd, **kwds):
         rsp = srv.cmd(cmd, kwds)
         if 'error' in rsp:
-            raise Exception(rsp['error']['desc'])
+            raise QMPError(rsp['error']['desc'])
         return rsp['return']
 
     rsp = do_command(srv, 'query-cpus')

bessctl/conf/port/vhost/qmp.py

@@ -195,7 +195,7 @@ def cmd(self, name, args=None, id=None):
     def command(self, cmd, **kwds):
         ret = self.cmd(cmd, kwds)
         if 'error' in ret:
-            raise Exception(ret['error']['desc'])
+            raise QMPError(ret['error']['desc'])
         return ret['return']
 
     def pull_event(self, wait=False):

bessctl/measurement_utils.py

@@ -51,7 +51,7 @@ def get_local_bess_handle():
     try:
         bess.connect()
     except BESS.RPCError:
-        raise Exception('BESS is not running')
+        raise ConnectionError('BESS is not running')
     return bess
 
 

bessctl/module_tests/url_filter.py

@@ -121,13 +121,6 @@ def test_urlfilter_selfconfig(self):
         ]}
         arg = pb_conv.protobuf_to_dict(uf.get_initial_arg())
         cur_config = pb_conv.protobuf_to_dict(uf.get_runtime_config())
-        # import pprint
-        # def pp2(*args):
-        #    for a, b in zip(*[iter(args)] * 2):
-        #        print('{}:'.format(a))
-        #        pprint.pprint(b, indent=4)
-        # pp2('iconf:', iconf, 'arg:', arg,
-        #    '\nmut state:', cur_config, 'expecting:', expect_config)
         assert arg == iconf and cur_config == expect_config
 
 suite = unittest.TestLoader().loadTestsFromTestCase(BessUrlFilterTest)

bessctl/run_module_tests.py

@@ -78,7 +78,7 @@ def main():
     try:
         run_cmd('%s daemon start -m 0' % bessctl)
     except CommandError:
-        raise Exception('bess daemon could not start')
+        raise RuntimeError('bess daemon could not start')
 
     for file_name in glob.glob(os.path.join(args.test_dir, "{}.py".format(args.test_name))):
         print('Running test %s' % file_name)

bessctl/static/graph.html

@@ -140,17 +140,13 @@
     <script>
       var timer = setTimeout(refresh, 1000);
 
-      function update_toast(selector, show_if) {
-        var elem = $(selector)
-        if (show_if) {
-          if (!elem.hasClass('show')) {
-            elem.prependTo('#toasts').toast('show');
-          }
-        } else {
-          if (elem.hasClass('show')) {
-            elem.toast('hide');
-          }
-        }
+      function update_toast(selector, show_if) {  
+        var elem = $(selector)  
+        if (show_if && !elem.hasClass('show')) {  
+          elem.prependTo('#toasts').toast('show');  
+        } else if (!show_if && elem.hasClass('show')) {  
+          elem.toast('hide');  
+        }  
       }
 
       function refresh() {

bessctl/static/pipeline.js

@@ -59,124 +59,192 @@ function add_datapoints(stats, module_name, gates, gate_type) {
     }
 }
 
-function get_edge_label(stats) {
-    const num_stats = stats.length;  
-    const value = stats[num_stats - 1];  
-    let label = '?'  
-
-    if (value.timestamp > 0) {
-        switch (opt_mode) {
-            case 'total':
-                label = value[opt_field];
-                break;
-            case 'rate':
-                if (num_stats >= 2) {
-                    var last = stats[num_stats - 2];
-                    var time_diff = value.timestamp - last.timestamp;
-                    if (opt_field == 'batchsize') {
-                        var packets = value.pkts - last.pkts;
-                        var batches = value.cnt - last.cnt;
-                        label = batches ? packets / batches : 'N/A';
-                    } else {
-                        var value_diff = value[opt_field] - last[opt_field];
-                        label = Math.round(value_diff / time_diff);
-                    }
-                }
-                break;
-            case 'none':
-                return '';
-            default:
-                throw new Error('Unknown mode ' + opt_mode);
-        }
-    }
-
-    if ((typeof label == 'number') && opt_humanreadable) {
-        let unit = ' ';
-        if (opt_mode == 'rate') {
-            if (label > 1000000000) {
-                label /= 1000000000;
-                unit += 'G';
-            } else if (label > 1000000) {
-                label /= 1000000;
-                unit += 'M';
-            } else if (label > 1000) {
-                label /= 1000;
-                unit += 'k';
-            }
-            if (opt_field == 'pkts') {
-                unit += 'pps';
-            } else if (opt_field == 'bits') {
-                unit += 'bps';
-            }
-        }
-        label = label.toLocaleString('en-US', {maximumFractionDigits: 2}) + unit;
-    }
-
-    // We need this HTML hack to give background color to edge labels
-    return `<<table border="0" cellpadding="0"><tr><td bgcolor="white">${label}</td></tr></table>>`;
+function get_edge_label(stats, options) {  
+    if (!stats || stats.length === 0) {  
+        return format_html_label('?');  
+    }  
+
+    const value = stats[stats.length - 1];  
+
+    if (value.timestamp <= 0) {  
+        return format_html_label('?');  
+    }  
+
+    const label = calculate_label_by_mode(stats, value, options);  
+
+    // Mode 'none' returns an empty string, which should not be wrapped in HTML
+    if (label === '') {  
+        return '';  
+    }  
+
+    const formatted_label = format_label_text(label, options);  
+    return format_html_label(formatted_label);  
+}  
+
+function calculate_label_by_mode(stats, value, options) {  
+    switch (options.mode) {  
+        case 'total':  
+            return value[options.field];  
+        case 'rate':  
+            return calculate_rate_label(stats, value, options);  
+        case 'none':  
+            return '';  
+        default:  
+            throw new Error('Unknown mode ' + options.mode);  
+    }  
+}  
+
+function calculate_rate_label(stats, value, options) {  
+    if (stats.length < 2) {  
+        return '?';  
+    }  
+
+    const last = stats[stats.length - 2];  
+    const time_diff = value.timestamp - last.timestamp;  
+
+    if (options.field === 'batchsize') {  
+        const packets = value.pkts - last.pkts;  
+        const batches = value.cnt - last.cnt;  
+        return batches ? packets / batches : 'N/A';  
+    }  
+
+    const value_diff = value[options.field] - last[options.field];  
+    return Math.round(value_diff / time_diff);  
+}  
+
+function format_label_text(label, options) {  
+    if (typeof label !== 'number' || !options.humanreadable) {  
+        return label;  
+    }  
+
+    if (options.mode === 'rate') {  
+        return format_rate_number(label, options.field);  
+    }  
+
+    return label.toLocaleString('en-US', {maximumFractionDigits: 2});  
+}  
+
+function format_rate_number(label, field) {  
+    let unit = ' ';  
+    let scaled = label;  
+
+    if (label > 1000000000) {  
+        scaled /= 1000000000;  
+        unit += 'G';  
+    } else if (label > 1000000) {  
+        scaled /= 1000000;  
+        unit += 'M';  
+    } else if (label > 1000) {  
+        scaled /= 1000;  
+        unit += 'k';  
+    }  
+
+    if (field === 'pkts') unit += 'pps';  
+    else if (field === 'bits') unit += 'bps';  
+
+    return scaled.toLocaleString('en-US', {maximumFractionDigits: 2}) + unit;  
+}  
+
+function format_html_label(label) {  
+    return `<<table border="0" cellpadding="0"><tr><td bgcolor="white">${label}</td></tr></table>>`;  
 }
 
-function graph_to_dot(modules) {
-    opt_field = document.querySelector('input[name="metric"]:checked').value;
-    opt_mode = document.querySelector('input[name="mode"]:checked').value;
-    opt_humanreadable = document.querySelector('input[name="humanreadable"]').checked;
-
-    let nodes = '';
-    for (const module_name in modules) {
-	const module = modules[module_name];
-	// no need to collect igate data since we don't show them yet.
-        // add_datapoints(stats, module_name, module.igates, 'igate')
-        add_datapoints(stats, module_name, module.ogates, 'ogate')
-
-        module.show_igates = module.igates.length > 1 ||
-            (module.igates.length == 1 && module.igates[0].igate != 0);
-        module.show_ogates = module.ogates.length > 1 ||
-            (module.ogates.length == 1 && module.ogates[0].ogate != 0);
-
-        const desc = module.desc ? `<font point-size="9">${module.desc}</font>` : '';
-        const igates = module.show_igates ? gates_to_str(module.igates, 'igate') : '';
-        const ogates = module.show_ogates ? gates_to_str(module.ogates, 'ogate') : '';
-
-        nodes += `
-  "${module_name}" [shape=plaintext label=
-    <<table port="mod" border="1" cellborder="0" cellspacing="0" cellpadding="1">
-      ${igates}<tr>
-        <td width="60">${module_name}</td>
-      </tr>
-      <tr>
-        <td><font color="#888888" point-size="9"><i>${module.mclass}</i></font></td>
-      </tr>
-      <tr>
-        <td>${desc}</td>
-      </tr>
-      ${ogates}</table>>];
-`;
-    }
-
-    let edges = '';
-    for (module_name in modules) {
-        const module = modules[module_name];
-        for (let i = 0; i < module.ogates.length; i++) {
-            const gate = module.ogates[i];
-            const dst_module = modules[gate.name];
-            const out_port = module.show_ogates ? `ogate${gate.ogate}:s` : 'mod';
-            const in_port = dst_module.show_igates ? `igate${gate.igate}:n` : 'mod';
-
-            let label = get_edge_label(stats[[module_name, 'ogate', gate.ogate]]);
-            if (label != '') {
-                label = ` [label=${label}]`;
-            }
-
-            edges += `  "${module_name}":${out_port} -> "${gate.name}":${in_port}${label}\n`;
-        }
-    }
-
-    return `digraph G {
-  graph [ rankdir=TB ];
-  node [ fontsize=12 ];
-  edge [ fontsize=9, color="#ffb30f", arrowsize=0.5, labeldistance=1.2 ];
-${nodes}
-${edges}
-}
-`
+function graph_to_dot(modules) {  
+    const options = get_graph_options();  
+    // Pass options down so sub-functions can use them
+    const nodes = generate_nodes(modules, options);  
+    const edges = generate_edges(modules, options);  
+
+    return `digraph G {  
+  graph [ rankdir=TB ];  
+  node [ fontsize=12 ];  
+  edge [ fontsize=9, color="#ffb30f", arrowsize=0.5, labeldistance=1.2 ];  
+${nodes}  
+${edges}  
+}  
+`;  
+}  
+
+function get_graph_options() {  
+    return {  
+        field: document.querySelector('input[name="metric"]:checked').value,  
+        mode: document.querySelector('input[name="mode"]:checked').value,  
+        humanreadable: document.querySelector('input[name="humanreadable"]').checked  
+    };  
+}  
+
+function generate_nodes(modules, options) {  
+    let nodes = '';  
+    for (const module_name in modules) {  
+        const module_data = modules[module_name];  
+
+        // Pass options if add_datapoints needs them
+        add_datapoints(stats, module_name, module_data.ogates, 'ogate');  
+
+        set_gate_visibility(module_data);  
+
+        // Pass options if gates_to_str needs them
+        const node_content = create_module_node(module_data, module_name, options);  
+        nodes += node_content;  
+    }  
+    return nodes;  
+}  
+
+function set_gate_visibility(module) {  
+    const check = (gates, type) => gates.length > 1 || (gates.length === 1 && gates[0][type] !== 0);
+    module.show_igates = check(module.igates, 'igate');  
+    module.show_ogates = check(module.ogates, 'ogate');  
+}  
+
+function create_module_node(module, module_name, options) {  
+    const desc = module.desc ? `<font point-size="9">${module.desc}</font>` : '';  
+    // Original gates_to_str might need options
+    const igates = module.show_igates ? gates_to_str(module.igates, 'igate', options) : '';  
+    const ogates = module.show_ogates ? gates_to_str(module.ogates, 'ogate', options) : '';  
+
+    return `  
+  "${module_name}" [shape=plaintext label=  
+    <<table port="mod" border="1" cellborder="0" cellspacing="0" cellpadding="1">  
+      ${igates}<tr>  
+        <td width="60">${module_name}</td>  
+      </tr>  
+      <tr>  
+        <td><font color="#888888" point-size="9"><i>${module.mclass}</i></font></td>  
+      </tr>  
+      <tr>  
+        <td>${desc}</td>  
+      </tr>  
+      ${ogates}</table>>];  
+`;  
+}  
+
+function generate_edges(modules, options) {  
+    let edges = '';  
+    for (const module_name in modules) {  
+        const module = modules[module_name];  
+        edges += create_module_edges(module, module_name, modules, options);  
+    }  
+    return edges;  
+}  
+
+function create_module_edges(module, module_name, modules, options) {  
+    let edges = '';  
+    for (const gate of module.ogates) {  
+        const dst_module = modules[gate.name];  
+        edges += create_single_edge(module, module_name, gate, dst_module, options);  
+    }  
+    return edges;  
+}  
+
+function create_single_edge(module, module_name, gate, dst_module, options) {  
+    const out_port = module.show_ogates ? `ogate${gate.ogate}:s` : 'mod';  
+    const in_port = dst_module.show_igates ? `igate${gate.igate}:n` : 'mod';  
+
+    // Pass options to get_edge_label
+    let label = get_edge_label(stats[[module_name, 'ogate', gate.ogate]], options);  
+    if (label !== '') {  
+        label = ` [label=${label}]`;  
+    }  
+
+    return `  "${module_name}":${out_port} -> "${gate.name}":${in_port}${label}\n`;  
 }