handle enable shell, wrapper for swssconfig from config #3
Conversation
|
This pull request introduces 5 alerts when merging 654aed1 into 3e52604 - view on LGTM.com new alerts:
|
config/main.py
Outdated
|
|
||
| @switch.command('shell') | ||
| @click.pass_context | ||
| @click.argument('en', metavar='<en>', required=True, type=int) |
There was a problem hiding this comment.
How the CLI will look? will it be en=0 or en=1?
config/main.py
Outdated
| with open (tmp_file) as f: | ||
| cfg = json.load(f) | ||
|
|
||
| cfg[0]["SWITCH_TABLE:switch"]["sdk_diag_shell"] = en |
There was a problem hiding this comment.
can you handle try catch if switch.json doesn't have "sdk_diag_shell"?
There was a problem hiding this comment.
py is appending this new KV pair to the list.
And swssconfig will handle unsupported attributes
config/main.py
Outdated
| """switch related config""" | ||
| pass | ||
|
|
||
| @switch.command('shell') |
There was a problem hiding this comment.
Can we have more specific command , like "diag_shell" or "sdk_shell" ?
config/main.py
Outdated
|
|
||
| @switch.command('shell') | ||
| @click.pass_context | ||
| @click.argument('en', metavar='<en>', required=True, type=int) |
There was a problem hiding this comment.
Instead of "int" arg, we can have it as choice arg,
@click.argument('sdk_shell', required=True, type=click.Choice(['enable', 'disable']))
|
This pull request introduces 1 alert when merging 69731c0 into 3e52604 - view on LGTM.com new alerts:
|
- 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)