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

feat: improved CLI and configuration handling #1955

Closed
teto opened this issue Apr 3, 2024 · 3 comments · Fixed by #1974
Closed

feat: improved CLI and configuration handling #1955

teto opened this issue Apr 3, 2024 · 3 comments · Fixed by #1974
Assignees
Labels
enhancement New feature or request

Comments

@teto
Copy link

teto commented Apr 3, 2024

Is your feature request related to a problem? Please describe.

everytime I have to run localai, I have to pass it several flags like the listenAddress, the models-path and so on. These rarely changes and I would like to just record them so that I can just type localai.
Some (all?) of them can be passed via environment variables but I dont like that, it pollutes the env (especially as some variables are not even prefixed with LOCALAI, is more implicit. Having a config.yaml I could edit would be much better. I just found out about --localai-config-dir` but it doesn't seem to be able to load the models-path for instance ? I had a look at the code and I think localai just watches that folder for api keys and tokens ?

Describe the solution you'd like
localai loads its flags/configuration from a configuration file (e.g. config.yaml). Ideally localai would look into $XDG_CONFIG_HOME/localai (e.g. ~/.config/localai/)

Describe alternatives you've considered

Export all the environment variables.
One could also create a wrapper or alias but because I intend to hack on localai, that might not be easy either

Additional context

I also find the --help for --localai-config-dir not descriptive enough of what it does

@teto teto added the enhancement New feature or request label Apr 3, 2024
@cryptk
Copy link
Collaborator

cryptk commented Apr 4, 2024

Currently LocalAI uses the urfave/cli package to build the CLI for the application. It looks like it can be extended pretty easily to allow loading the config from a YAML file: https://cli.urfave.org/v2/examples/flags/#values-from-alternate-input-sources-yaml-toml-and-others.

Unfortunately after testing this route, it appears that if we provide a default value for the config file location using the altsrc functionality, that makes the configuration file mandatory. While we might be able to work around this by creating a closure that wraps around the required functionality checks for the presence of the file first, I'm curious if that time may be better spent by moving the project to something like cobra and viper for CLI and configuration handling.

For what it's worth, if we do NOT provide a default file path, then everything works as you would expect, but it means that in order to use a config file, your invocation would need to be something like ./local-ai --yamlConfig /path/to/config.yaml as opposed to being able to just call ./local-ai and having it figure it out.

Going to wait on this one until I get some clarification from @mudler about which way he would prefer we go with this one.

@mudler
Copy link
Owner

mudler commented Apr 4, 2024

I do agree a form of configuration either YAML, json, TOML (or all of them?) makes totally sense.

Additional context

I also find the --help for --localai-config-dir not descriptive enough of what it does

Good point, first we should improve documentation/help around that

Currently LocalAI uses the urfave/cli package to build the CLI for the application. It looks like it can be extended pretty easily to allow loading the config from a YAML file: https://cli.urfave.org/v2/examples/flags/#values-from-alternate-input-sources-yaml-toml-and-others.

Unfortunately after testing this route, it appears that if we provide a default value for the config file location using the altsrc functionality, that makes the configuration file mandatory. While we might be able to work around this by creating a closure that wraps around the required functionality checks for the presence of the file first, I'm curious if that time may be better spent by moving the project to something like cobra and viper for CLI and configuration handling.

I have a mixed feeling on viper/cobra, almost all my other projects used previously viper/cobra, but it ended it to be always more complex for what was actually the need/use case and I ended up always regretting it.

I'm not against it, however I'd be careful in considering it first, especially if urfave/cli already supports loading from other files.

There are also out there good alternatives, which might be worth considering too, I know at least 2 of them:

A side note: I think manual, obvious wrappers are fine and good enough to maintain as tends to be more readable then having hidden complexities which anyone would need to learn a framework first before being able to understand.

@cryptk
Copy link
Collaborator

cryptk commented Apr 4, 2024

So far, I think I'm liking Kong more just from reading through their docs. It also appears to have good support for loading from a config file, has sub-commands, flags, envvars (including a nice example using godotenv to support loading from a .env file), etc. So it should cover everything we are currently using urfave/cli for, but with better support for config file loading (specifically not dying if a default config file path does not exist).

@cryptk cryptk self-assigned this Apr 5, 2024
@cryptk cryptk changed the title make --localai-config-dir help clearer feat: improved CLI and configuration handling Apr 5, 2024
mudler pushed a commit that referenced this issue Apr 11, 2024
* feat: migrate to alecthomas/kong for CLI

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* feat: bring in new flag for granular log levels

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* chore: go mod tidy

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* feat: allow loading cli flag values from ["./localai.yaml", "~/.config/localai.yaml", "/etc/localai.yaml"] in that order

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* feat: load from .env file instead of a yaml file

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* feat: better loading for environment files

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* feat(doc): add initial documentation about configuration

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* fix: remove test log lines

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* feat: integrate new documentation into existing pages

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* feat: add documentation on .env files

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* fix: cleanup some documentation table errors

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

* feat: refactor CLI logic out to it's own package under core/cli

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>

---------

Signed-off-by: Chris Jowett <421501+cryptk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants