[multi-asic][warm_restart] add Multi-ASIC support for warm_restart commands#99
[multi-asic][warm_restart] add Multi-ASIC support for warm_restart commands#99stepanblyschak wants to merge 2 commits intomasterfrom
Conversation
stepanblyschak
force-pushed
the
masic-warm-restart
branch
4 times, most recently
from
6e8a875 to
2fa8538
Compare
stepanblyschak
changed the title
stepanblyschak
force-pushed
the
masic-warm-restart
branch
from
2fa8538 to
3534a86
Compare
stepanblyschak
force-pushed
the
masic-warm-restart
branch
from
3534a86 to
a557b59
Compare
| state_db = ctx.obj['state_db'] | ||
| config_db = ctx.obj['db'] | ||
| def warm_restart_enable(ctx, namespace, module): | ||
| if namespace is not None: |
There was a problem hiding this comment.
minor: if namespace and namespace not in ctx.obj["all_namespaces"]
There was a problem hiding this comment.
need check None, which means user did not provide. '' - is global ns
|
|
||
|
|
||
| @warm_restart.command('enable') | ||
| @click.option('--namespace', '-n', 'namespace', default=None, help='Namespace name') |
There was a problem hiding this comment.
If you use this decorator instead, you won't namespace validation inside funtion:
@click.option('-n', '--namespace', help='Namespace name', required=True if multi_asic.is_multi_asic() else False,
type=click.Choice(multi_asic.get_namespace_list()), default=multi_asic.DEFAULT_NAMESPACE)
There was a problem hiding this comment.
This will reduce the number of changes in this file
There was a problem hiding this comment.
@oleksandrivantsiv I see the benefit of doing this, but not sure how to mock multi_asic functions later in a test case. Since this decorator runs on module import time, it likelly needs reloading module after mock setup. I tried this but seems this does not play well with ClickRunner I will check what I can do.
There was a problem hiding this comment.
@oleksandrivantsiv I find it less suitable for this command. The choice enumerates asic namesapces while my commands use global and asic namespaces. Also makes mocking harder
The only benefit of choice is CLI feedback, but this is not a user facing CLI
stepanblyschak
force-pushed
the
masic-warm-restart
branch
from
a557b59 to
0063970
Compare
Signed-off-by: Stepan Blyschak <[email protected]>
Signed-off-by: Stepan Blyschak <[email protected]>
stepanblyschak
force-pushed
the
masic-warm-restart
branch
from
0063970 to
68607ee
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)