Skip to content

Commit 2d9d00d

Browse files
authored
[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.
1 parent 84b2c5b commit 2d9d00d

File tree

1 file changed

+26
-19
lines changed

1 file changed

+26
-19
lines changed

config/main.py

+26-19
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,6 @@
5555

5656
asic_type = None
5757

58-
#
59-
# Load breakout config file for Dynamic Port Breakout
60-
#
61-
62-
try:
63-
(platform, hwsku) = device_info.get_platform_and_hwsku()
64-
except Exception as e:
65-
click.secho("Failed to get platform and hwsku with error:{}".format(str(e)), fg='red')
66-
raise click.Abort()
67-
68-
try:
69-
breakout_cfg_file = get_port_config_file_name(hwsku, platform)
70-
except Exception as e:
71-
click.secho("Breakout config file not found with error:{}".format(str(e)), fg='red')
72-
raise click.Abort()
73-
7458
#
7559
# Breakout Mode Helper functions
7660
#
@@ -84,11 +68,32 @@ def readJsonFile(fileName):
8468
raise Exception(str(e))
8569
return result
8670

87-
def _get_option(ctx,args,incomplete):
71+
def _get_breakout_cfg_file_name():
72+
"""
73+
Get name of config file for Dynamic Port Breakout
74+
"""
75+
try:
76+
(platform, hwsku) = device_info.get_platform_and_hwsku()
77+
except Exception as e:
78+
click.secho("Failed to get platform and hwsku with error:{}".format(str(e)), fg='red')
79+
raise click.Abort()
80+
81+
try:
82+
breakout_cfg_file_name = get_port_config_file_name(hwsku, platform)
83+
except Exception as e:
84+
click.secho("Breakout config file not found with error:{}".format(str(e)), fg='red')
85+
raise click.Abort()
86+
87+
return breakout_cfg_file_name
88+
89+
90+
def _get_breakout_options(ctx, args, incomplete):
8891
""" Provides dynamic mode option as per user argument i.e. interface name """
8992
all_mode_options = []
9093
interface_name = args[-1]
9194

95+
breakout_cfg_file = _get_breakout_cfg_file_name()
96+
9297
if not os.path.isfile(breakout_cfg_file) or not breakout_cfg_file.endswith('.json'):
9398
return []
9499
else:
@@ -2140,14 +2145,16 @@ def speed(ctx, interface_name, interface_speed, verbose):
21402145

21412146
@interface.command()
21422147
@click.argument('interface_name', metavar='<interface_name>', required=True)
2143-
@click.argument('mode', required=True, type=click.STRING, autocompletion=_get_option)
2144-
@click.option('-f', '--force-remove-dependencies', is_flag=True, help='Clear all depenedecies internally first.')
2148+
@click.argument('mode', required=True, type=click.STRING, autocompletion=_get_breakout_options)
2149+
@click.option('-f', '--force-remove-dependencies', is_flag=True, help='Clear all dependencies internally first.')
21452150
@click.option('-l', '--load-predefined-config', is_flag=True, help='load predefied user configuration (alias, lanes, speed etc) first.')
21462151
@click.option('-y', '--yes', is_flag=True, callback=_abort_if_false, expose_value=False, prompt='Do you want to Breakout the port, continue?')
21472152
@click.option('-v', '--verbose', is_flag=True, help="Enable verbose output")
21482153
@click.pass_context
21492154
def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load_predefined_config):
21502155
""" Set interface breakout mode """
2156+
breakout_cfg_file = _get_breakout_cfg_file_name()
2157+
21512158
if not os.path.isfile(breakout_cfg_file) or not breakout_cfg_file.endswith('.json'):
21522159
click.secho("[ERROR] Breakout feature is not available without platform.json file", fg='red')
21532160
raise click.Abort()

0 commit comments

Comments
 (0)