fal-ai / fal

⚡ Fastest way to serve open source ML models to millions
https://fal.ai/docs
Apache License 2.0
509 stars 44 forks source link

feat(cli): allow using --debug on any subcommand #136

Closed efiop closed 6 months ago

efiop commented 6 months ago

Followup for reverted #118

This is a conventional click approach for shared flags.

Looked into adding tests, but the new approach is declarative in itself, so declarative tests seem redundant (e.g. testing --help). We still need real cli tests though (e.g. to make sure things like fal fn run ... work), but that is out of scope for this PR and will be done in a followup.

linear[bot] commented 6 months ago

FEA-2110 fal cli doesn't support --debug on all subcommands

efiop commented 6 months ago

@chamini2 Just development comfort, tbh. E.g. say I'm running fal fn run myfile.py myfunc and something goes wrong. It is natural to just hit up-arrow and append --debug and have it work instead of having to navigate to the beginning of the command and insert --debug there.

--debug is really just first one that is good to have, but we will find ourselves adding more convenience options for both internal and external users. E.g. from the top of my head something like a hidden (or not) --cprofile option to profile our own fal commands.

On the grand scale of things this is pretty minor, just found it bothering me while playing with it.

efiop commented 6 months ago

Is there no way to have this option be inherited by all commands without needing to explicitly decorate the functions? Otherwise I fear we will forget to decorate a func one day

I obviously did try to avoid it, but alternatives (e.g. creating a FalCommand and having it automatically insert common options, and also setting it as group.command_class and so on) were not fitting the click philosophy and were turning into a bigger maintenance mess. The current way is explicit but maintainable in a conventional way for anyone without surprises. Once we have more common options I'll combine them into one decorator for convenience and will take another look into dedup.