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

WIP cli tool #44

Open
wants to merge 45 commits into
base: main
Choose a base branch
from
Open

WIP cli tool #44

wants to merge 45 commits into from

Conversation

tbarbugli
Copy link
Member

@tbarbugli tbarbugli commented Jun 24, 2024

  • Supports nested commands based on product (cli chat create_channel, cli video create_call, cli create_token)
  • Get credentials from args or env vars
  • Automatically generate all CRUD commands based on a index (see video.py)
  • Wrap cli in a docker image so its possible to run it with docker run without using python locally
  • Look into pipx to see if that's a good idea to use it as well (bit faster than docker probably)
  • Update examples in docs to use the CLI (probably better than the curl examples)
  • Find a way to test that commands are working (at least the shared / utils code and some basic stuff)

Examples:

STREAM_API_KEY=x STREAM_API_SECRET=z poetry run cli video rtmp-in-setup
STREAM_API_KEY=x STREAM_API_SECRET=z poetry run cli create-token \
   --user-id tommaso --exp-seconds 60 \
   --call-cid default:a \
   --call-cid default:b --role banana

Missing stuff:
image

Once we have this, we can have commands like these work out of the box and without any manual work

JSON_DATA='{
  "created_by_id": "tommaso",
  "custom": {"color": "blue"}
}'

STREAM_API_KEY=x STREAM_API_SECRET=z poetry run cli video get-or-create 
   --call-type default 
   --call-id banana
   --members-limit 10
   --data $JSON_DATA


return update_wrapper(new_func, f)

def json_option(option_name):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this do? lets add a docstring with example


return cmd

def add_option(cmd, param_name, param):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add_option_from_arg is bit easier to understand

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about optional vs not optional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

highly suggested to write a bunch of unit tests for this function (fastest way to make it 100%)

cmd = click.option(f'--{param_name}', type=int)(cmd)
elif param.annotation == bool:
cmd = click.option(f'--{param_name}', is_flag=True)(cmd)
elif param.annotation == list:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably not good enough: a list of strings or a list of numbers can be handled with click options multiple. A list of lists or structs needs to be provided as json encoded string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants