-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
issue#20 Improve CLI operations #43
base: master
Are you sure you want to change the base?
Changes from 2 commits
35b09d7
38a1f26
6fdc39b
ea89757
2efc341
adc732b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,32 @@ | |
"close": "stop", | ||
"connect": "start", | ||
} | ||
# argument should be a string | ||
ARGS = { | ||
"path": "/path:str", | ||
"data": "data:str", | ||
} | ||
|
||
# flag should be a set of its representations (strings) | ||
KWARGS = { | ||
"ephemeral": {"-e", "--ephemeral"}, | ||
"sequence": {"-s", "--sequence"}, | ||
"boolean1": {"-b1", "--boolean1"}, | ||
"boolean2": {"-b2", "--boolean2"}, | ||
} | ||
|
||
# program can distinguish kwarg and arg by their type | ||
CMD = { | ||
# create /test 0x0 False False | ||
"create": [ARGS["path"], ARGS["data"], KWARGS["ephemeral"], KWARGS["sequence"]], | ||
# test /test False True 0x0 | ||
"test": [ARGS["path"], KWARGS["boolean1"], KWARGS["boolean2"], ARGS["data"]], | ||
|
||
# add more commands format here for parsing... | ||
} | ||
# parse status code | ||
PARSE_SUCCESS = 1 | ||
PARSE_ERROR = 0 | ||
|
||
fkCompleter = WordCompleter(keywords, ignore_case=True) | ||
|
||
|
@@ -137,6 +163,46 @@ def process_cmd(client: FaaSKeeperClient, cmd: str, args: List[str]): | |
return client.session_status, client.session_id | ||
|
||
|
||
# find the position of the argument in the command (function call like), return -1 if the argument is not a flag | ||
def kwarg_pos(arg: str, kwargs_array: List[str]): | ||
for idx, kwarg in enumerate(kwargs_array): | ||
if (type(kwarg) is set) and (arg in kwarg): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please use |
||
return idx | ||
return -1 | ||
|
||
# print the command format and flags to user | ||
def print_cmd_info(cmd: str): | ||
args = "" | ||
flags = "" | ||
for arg in CMD[cmd]: | ||
if type(arg) is str: | ||
args += f"{arg} " | ||
else: | ||
flags += f"{arg} " | ||
click.echo(f"Command: {args}| Flags: {flags}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can provide users with examples that are syntactically correct. e.g. |
||
|
||
|
||
# parse the arguments and return the parsed arguments | ||
# status code: 0 - success, 1 - not need to parse, 2 - error | ||
def parse_args(cmd: str, args: List[str]): | ||
if cmd not in CMD: | ||
return args, PARSE_SUCCESS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Im a bit confused by the Status code, PARSE_SUCCESS is used here and at the end of a successful mapping. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My mistake, for now "PARSE_SUCCESS" is useless. I'm considering whether to put input check in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion, parser/mapper would map user inputs to the defined syntax amd let There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought we needed to allow [bool_arguments] to appear in any position... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
My bad, I test with zookeeper and it does allow [bool_arguments] to be anywhere. You are right, thank you for pointing that out. |
||
else: | ||
parsed_args = [False] * len(CMD[cmd]) | ||
arg_idx = parse_args_idx = 0 | ||
while arg_idx < len(args): | ||
idx = kwarg_pos(args[arg_idx], CMD[cmd]) | ||
if idx != -1: | ||
parsed_args[idx] = True | ||
else: | ||
while isinstance(CMD[cmd][parse_args_idx], set): # skip the positions for flags | ||
parse_args_idx += 1 | ||
parsed_args[parse_args_idx] = args[arg_idx] | ||
parse_args_idx += 1 | ||
arg_idx += 1 | ||
return parsed_args, PARSE_SUCCESS | ||
|
||
|
||
@click.command() | ||
@click.argument("config", type=click.File("r")) | ||
@click.option("--port", type=int, default=-1) | ||
|
@@ -190,7 +256,9 @@ def cli(config, port: int, verbose: str): | |
elif cmd not in keywords: | ||
click.echo(f"Unknown command {text}") | ||
else: | ||
status, session_id = process_cmd(client, cmd, cmds[1:]) | ||
print(cmd, cmds[1:]) | ||
parsed_args = parse_args(cmd, cmds[1:]) | ||
status, session_id = process_cmd(client, cmd, parsed_args) | ||
counter += 1 | ||
|
||
print("Closing...") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since
def parse_args(cmd: str, args: List[str])
looks like have a flexibility to support other cmds likeget_data
andset_data
, please add them.