search

livekit / livekit-cli

Command line interface to LiveKit
https://docs.livekit.io
Other
215 stars 65 forks source link

Support new SIP Trunk API. Move to subcommands. #340

Closed dennwc closed 3 months ago

dennwc commented 3 months ago

Support new SIP Inbound/Outbound Trunk API and move all SIP commands into a separate cli tree:

Old commands continue to work, they are only hidden from the CLI help.

I also renamed the columns for legacy trunks list command. Now they match names in new API (OutboundNumber -> Number, InboundNumbers -> AllowedNumbers).

Requires https://github.com/livekit/protocol/pull/738

dennwc commented 3 months ago

This should wait for prod deployment. It hides old CLI commands, while new ones won't work without updated server.

davidzhao commented 3 months ago

@rektdeckard what do you think about the subcommand organization? this is something we'd also want to make consistent across the board.

dennwc commented 3 months ago

Regarding sub-commands, it would be also nice to move shared options like URL/key/secret to the root command:

livekit-cli --api-key <key> --api-secret <secret> some sub command

instead of

livekit-cli some sub command  --api-key <key> --api-secret <secret>
rektdeckard commented 3 months ago

@davidzhao @dennwc looks like what what we had in mind, and consistent with prior art in the projects subcommand. Great shout on moving the global parameters too! Other main thing to think about IMO is making the key argument a positional argument, for example:

livekit-cli room join "my-first-room"

instead of

livekit-cli join-room --room "my-first-room"

Since the room is the main and only logical receiver of the command. I don't know if the --request flag qualifies here or not. WDYT?

dennwc commented 3 months ago

Good idea! Will make main arg positional then for new SIP commands :+1:

Regarding flags, that should probably be in a separate PR, because existing commands will need to be updated too.

dennwc commented 3 months ago

@rektdeckard Updated new SIP commands to accept main args (id or request).

Moving flags to the top will require an update to github.com/urfave/cli/v3, which added Persistent to the flag definitions. Could be a separate PR.

rektdeckard commented 3 months ago

Nice! Agree, separate PR for that. v2 seems to be in maintenance mode, so probably a good thing regardless?