Skip to content
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

Improve CLI operations #20

Open
mcopik opened this issue Feb 16, 2023 · 11 comments
Open

Improve CLI operations #20

mcopik opened this issue Feb 16, 2023 · 11 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@mcopik
Copy link
Collaborator

mcopik commented Feb 16, 2023

We implemented FaaSKeeper CLI by mapping user input to function arguments. This is not ideal - for example, we cannot skip some parameters, and we require users to pass "True" and "False" for boolean arguments.

On the other hand, ZooKeeper's commands are much nicer, e.g. create -e /ephemeral_node mydata instead of us using True for the ephemeral parameter.

@mcopik mcopik added enhancement New feature or request good first issue Good for newcomers labels Feb 16, 2023
@afzal442
Copy link

Hi @mcopik! I am interested in this issue. Can you provide some suggestion on how to get started and reproduce it? Thanks

@shailendrasingh117
Copy link

heyy!! @mcopik , I'm interested in this issue.

@mcopik
Copy link
Collaborator Author

mcopik commented Apr 1, 2023

@afzal442 @shailendrasingh117 Glad to hear you're interested!

Right now, if you use our CLI tool bin/fkCli.py, you need to supply all arguments of the command like calling a Python function. For example, if you want to create a new node /test with data 0x0, you need to call:

create /test 0x0 False False

Where both False correspond to creating sequential and ephemeral nodes - in this call, the node is neither sequential nor ephemeral. On the other hand, the same command works much simpler in the ZooKeeper CLI:

# same as above
create /test 0x0 
# creating an ephemeral and sequential node
create -s -e /test 0x0 

Our behavior should be similar to ZooKeeper. Right now, we take the arguments provided by the user, convert them to Python values, and call the create_node function of the Python API. Instead, we should provide a more user-friendly mapping of parameters.

@afzal442
Copy link

afzal442 commented Apr 2, 2023

Hello @mcopik ! so instead of boolean args, a user can pass -e and -s both or anyone or none to behave the same as abv. LMK! Thanks

@mcopik
Copy link
Collaborator Author

mcopik commented Apr 2, 2023

@afzal442 Correct! right now, we want to support create, get_data and set_data, but we should have a generic mapping structure that can be easily extended to different commands :) What is really needed, it's a simple parser of the command that maps flags and user inputs into our Python API.

@afzal442
Copy link

afzal442 commented Apr 2, 2023

    # Python types
    args_map = {
        "path": str,
        "data": bytes,
        "ephemeral": bool,
        "sequence": bool,
    }

    # Convert the arguments to the appropriate Python types
    kwargs = {}
    for arg in args:
        if arg in ("-e", "--ephemeral"):
            kwargs["ephemeral"] = True
        elif arg in ("-s", "--sequence"):
            kwargs["sequence"] = True
        else:
            try:
                key, value = arg.split("=")
            except ValueError:
                click.echo(f"Invalid argument: {arg}")
                return client.session_status, client.session_id
            if key not in args_map:
                click.echo(f"Invalid argument: {key}")
                return client.session_status, client.session_id
            kwargs[key] = args_map[key](value)

@shailendrasingh117
Copy link

   import click

  # Define the commands that can be parsed
     COMMANDS = {
      "create": {"args": ["path", "data"], "kwargs": ["ephemeral", "sequence"]},
      "get_data": {"args": ["path"], "kwargs": []},
      "set_data": {"args": ["path", "data"], "kwargs": []},
     }

   # Define the mapping between user inputs and Python types
      ARGS_MAP = {
     "path": str,
    "data": bytes,
    "ephemeral": bool,
    "sequence": bool,
    }

   # Define the CLI command for the parser
   @click.command()
   @click.argument("command")`
   @click.argument("args", nargs=-1)
   def cli(command, args):
   if command not in COMMANDS:
       click.echo(f"Invalid command: {command}")
       return
   cmd_info = COMMANDS[command]
   kwargs = {}
   i = 0
   for arg in cmd_info["args"]:
       if i >= len(args):
           click.echo(f"Missing argument: {arg}")
           return
       kwargs[arg] = ARGS_MAP[arg](args[i])
       i += 1
   for arg in cmd_info["kwargs"]:
       kwargs[arg] = False
   for arg in args[i:]:
       if arg.startswith("-"):
           if arg[1:] not in cmd_info["kwargs"]:
               click.echo(f"Invalid flag: {arg}")
               return
           kwargs[arg[1:]] = True
       else:
           click.echo(f"Invalid argument: {arg}")
           return
   if command == "create":
       create_node(**kwargs)
   elif command == "get_data":
       get_data(**kwargs)
   elif command == "set_data":
       set_data(**kwargs)
   else:
       click.echo(f"Unknown command: {command}")

`

    # Python types
    args_map = {
        "path": str,
        "data": bytes,
        "ephemeral": bool,
        "sequence": bool,
    }

    # Convert the arguments to the appropriate Python types
    kwargs = {}
    for arg in args:
        if arg in ("-e", "--ephemeral"):
            kwargs["ephemeral"] = True
        elif arg in ("-s", "--sequence"):
            kwargs["sequence"] = True
        else:
            try:
                key, value = arg.split("=")
            except ValueError:
                click.echo(f"Invalid argument: {arg}")
                return client.session_status, client.session_id
            if key not in args_map:
                click.echo(f"Invalid argument: {key}")
                return client.session_status, client.session_id
            kwargs[key] = args_map[key](value)

``

@shailendrasingh117
Copy link

Hey, I made some changes to the code you provided earlier. Would you mind taking a look and letting me know if everything looks okay? @mcopik

@mcopik
Copy link
Collaborator Author

mcopik commented Apr 3, 2023

@afzal442 @shailendrasingh117 Thanks for the contributions! Can you please open a PR with the code changes? I would like to be able to test that :-)

@THLiu55
Copy link

THLiu55 commented Mar 18, 2024

Hi @mcopik, I opened a PR with some changes. Would you mind taking a look and letting me know if everythink looks ok?

@mcopik
Copy link
Collaborator Author

mcopik commented Mar 20, 2024

@THLiu55 Thank you! We're reviewing this with @EricPyZhou and I hope it will be merged soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants