spcl / faaskeeper

A fully serverless implementation of the ZooKeeper coordination protocol.
BSD 3-Clause "New" or "Revised" License
16 stars 13 forks source link

Improve CLI operations #20

Open mcopik opened 1 year ago

mcopik commented 1 year ago

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.

afzal442 commented 1 year ago

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

shailendrasingh117 commented 1 year ago

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

mcopik commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago
    # 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 commented 1 year ago

   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 commented 1 year ago

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 commented 1 year ago

@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 commented 4 months ago

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

mcopik commented 4 months ago

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