Conversation
config/main.py
Outdated
| config_db = ConfigDBConnector() | ||
| config_db.connect() | ||
| ctx.obj = {} | ||
| ctx.obj['config_db'] = config_db |
There was a problem hiding this comment.
@abelamit use @clicommon.pass_db instead. Decorator is used only for the last chain. No need to keep the context for the entire CLI tree. If no code is present - use pass
| @click.option('--threshold', '--thresh', type=int, | ||
| help='Configure threshold for Tx error state - for all ports') | ||
| @click.option('--time_period', '--time', type=int, | ||
| help='Configure time period for Tx error state - for all ports') |
There was a problem hiding this comment.
@abelamit the best practice in CLI coding is to use the single (first) letter of the entire word, or a capital letter if the duplication is present. If more then two key words are present, the shortcut would be two (first) letters for each of the words:
Example:
@click.option('-t', '--threshold')
@click.option('-T', '--time-period')
@click.option('-tp', '--time-period')The recommended notation is:
@click.option("-t", "--threshold")
@click.option("-p", "--period")Useful stuff:
https://click.palletsprojects.com/en/stable/api/#types
There was a problem hiding this comment.
Fixed; thanks for the reference
|
|
||
| config_db = ValidatedConfigDBConnector(ctx.obj['config_db']) | ||
|
|
||
| if threshold is not None: |
There was a problem hiding this comment.
@abelamit FYI. The option validation can be done also using a dedicated validator object:
https://github.com/sonic-net/sonic-utilities/blob/master/config/plugins/sonic-trimming.py#L110
I would recommend to learn the existing solutions available in config/show dirs to master your Click framework skills
abelamit
force-pushed
the
master_amabel_utilities_lab5
branch
from
ed2333b to
454ef64
Compare
config/main.py
Outdated
| @clicommon.pass_db | ||
| def set_tx_error_status(ctx, threshold, period): | ||
| """Configure Tx error status threshold and/or time period""" | ||
| ctx = click.get_current_context() |
There was a problem hiding this comment.
@abelamit use @click.pass_context instead
config/main.py
Outdated
| @clicommon.pass_db | ||
| def del_tx_error_status(ctx, threshold, period): | ||
| """Delete configured Tx error status threshold and/or time period""" | ||
| ctx = click.get_current_context() |
There was a problem hiding this comment.
@abelamit use @click.pass_context instead
config/main.py
Outdated
| if threshold is None and period is None: | ||
| ctx.fail("Please provide either threshold or time period") | ||
|
|
||
| config_db = ValidatedConfigDBConnector(ctx.obj['config_db']) |
There was a problem hiding this comment.
@abelamit use @clicommon.pass_db instead
abelamit
force-pushed
the
master_amabel_utilities_lab5
branch
from
454ef64 to
b81cfca
Compare
2 participants
What I did
How I did it
How to verify it
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)