Skip to content

Commit

Permalink
[config] Eliminate port breakout-related globals (sonic-net#1045)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jleveque authored Aug 12, 2020
1 parent 84b2c5b commit 2d9d00d
Showing 1 changed file with 26 additions and 19 deletions.
45 changes: 26 additions & 19 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand All @@ -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:
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 2d9d00d

Please sign in to comment.