From 2d9d00d897d113603635bba64c30675bbd1976a6 Mon Sep 17 00:00:00 2001
From: Joe LeVeque <jleveque@users.noreply.github.com>
Date: Wed, 12 Aug 2020 11:50:08 -0700
Subject: [PATCH] [config] Eliminate port breakout-related globals (#1045)

Port breakout-related data should be gathered on-demand, not every time `config` is executed. If the ConfigDB is not ready, the call to get hwsku will hang indefinitely, which will occur when loading config for the first time. Also, if it fails to retrieve the port breakout globals, it will abort, even if the executed command was unrelated to port breakout. This is not desirable behavior.

This PR eliminates port breakout-related global variables, and encapsulates them in functions to be called on-demand, only when commands which require the data are executed. It is currently only accessed in two places. If we feel the need to cache it in the future for efficiency, we can look into adding it to the Click context.

Also rename `_get_option()` to `_get_breakout_cfg_file_name()` to add more detail.
---
 config/main.py | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/config/main.py b/config/main.py
index 6822077099b7..8b9443b20be2 100755
--- a/config/main.py
+++ b/config/main.py
@@ -55,22 +55,6 @@
 
 asic_type = None
 
-#
-# Load breakout config file for Dynamic Port Breakout
-#
-
-try:
-    (platform, hwsku) = device_info.get_platform_and_hwsku()
-except Exception as e:
-    click.secho("Failed to get platform and hwsku with error:{}".format(str(e)), fg='red')
-    raise click.Abort()
-
-try:
-    breakout_cfg_file = get_port_config_file_name(hwsku, platform)
-except Exception as e:
-    click.secho("Breakout config file not found with error:{}".format(str(e)), fg='red')
-    raise click.Abort()
-
 #
 # Breakout Mode Helper functions
 #
@@ -84,11 +68,32 @@ def readJsonFile(fileName):
         raise Exception(str(e))
     return result
 
-def _get_option(ctx,args,incomplete):
+def _get_breakout_cfg_file_name():
+    """
+    Get name of config file for Dynamic Port Breakout
+    """
+    try:
+        (platform, hwsku) = device_info.get_platform_and_hwsku()
+    except Exception as e:
+        click.secho("Failed to get platform and hwsku with error:{}".format(str(e)), fg='red')
+        raise click.Abort()
+
+    try:
+        breakout_cfg_file_name = get_port_config_file_name(hwsku, platform)
+    except Exception as e:
+        click.secho("Breakout config file not found with error:{}".format(str(e)), fg='red')
+        raise click.Abort()
+
+    return breakout_cfg_file_name
+
+
+def _get_breakout_options(ctx, args, incomplete):
     """ Provides dynamic mode option as per user argument i.e. interface name """
     all_mode_options = []
     interface_name = args[-1]
 
+    breakout_cfg_file = _get_breakout_cfg_file_name()
+
     if not os.path.isfile(breakout_cfg_file) or not breakout_cfg_file.endswith('.json'):
         return []
     else:
@@ -2140,14 +2145,16 @@ def speed(ctx, interface_name, interface_speed, verbose):
 
 @interface.command()
 @click.argument('interface_name', metavar='<interface_name>', required=True)
-@click.argument('mode', required=True, type=click.STRING, autocompletion=_get_option)
-@click.option('-f', '--force-remove-dependencies', is_flag=True,  help='Clear all depenedecies internally first.')
+@click.argument('mode', required=True, type=click.STRING, autocompletion=_get_breakout_options)
+@click.option('-f', '--force-remove-dependencies', is_flag=True,  help='Clear all dependencies internally first.')
 @click.option('-l', '--load-predefined-config', is_flag=True,  help='load predefied user configuration (alias, lanes, speed etc) first.')
 @click.option('-y', '--yes', is_flag=True, callback=_abort_if_false, expose_value=False, prompt='Do you want to Breakout the port, continue?')
 @click.option('-v', '--verbose', is_flag=True, help="Enable verbose output")
 @click.pass_context
 def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load_predefined_config):
     """ Set interface breakout mode """
+    breakout_cfg_file = _get_breakout_cfg_file_name()
+
     if not os.path.isfile(breakout_cfg_file) or not breakout_cfg_file.endswith('.json'):
         click.secho("[ERROR] Breakout feature is not available without platform.json file", fg='red')
         raise click.Abort()