spcl / faaskeeper

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

fix: Improves CLI operations for `ephemeral` and `sequential` params #31

Closed afzal442 closed 1 year ago

afzal442 commented 1 year ago

For #20

e.g.

[fk: 2023-04-03 19:47:24.456485 aws:faaskeeper-dev(CONNECTED) session:2ee45722 2] create /test 0x0 -e
['/test', '0x0', 'True', 'False']

cc @mcopik

mcopik commented 1 year ago

@afzal442 Thanks! The implementation seems pretty fixed for the arguments of the create operation, and it will be difficult to maintain and expand. After all, there are many more commands that we would like to support.

I think the alternative approach posted by @shailendrasingh117 will be more modular. Maybe you can both collaborate on a single solution?

afzal442 commented 1 year ago

Thanks @mcopik for the review! we can support for create operation by adding condition if cmd == `create` simply now.

Talking about the alternative approach, I had that same thing figured out but I don't know the use cases of get_data and set_data. That idea support for cmd is limited. What if cmd is sth else like connect if command not in COMMANDS: click.echo(f"Invalid command: {command}"). I'm not sure what it does and how it calls unkown methods.

       create_node(**kwargs)
   elif command == "get_data":
       get_data(**kwargs)
   elif command == "set_data":
       set_data(**kwargs)

LMK what you think.

shailendrasingh117 commented 1 year ago

This approach can also make it difficult to handle errors in a consistent way. If an unknown command is passed to the dispatcher, the current implementation simply prints an error message to the console. However, in a more complex application, it might be more appropriate to raise an exception or return an error code to the caller. @mcopik @afzal442

afzal442 commented 1 year ago

Closed as no progress