konveyor / kai

Konveyor AI - static code analysis driven migration to new targets via Generative AI
Apache License 2.0
23 stars 28 forks source link

:sparkles: Clean up and simplify config management #320

Closed JonahSussman closed 2 weeks ago

JonahSussman commented 2 weeks ago

Config is now sourced from the environment as well as the config.toml via pydantic-settings. This makes Kai align more with 12-factor config principles.

pydantic-settings allows for easy config overrides (higher overrides lower):

Task list:

JonahSussman commented 2 weeks ago

I updated how config priority is done. It's now done in the following way (higher overrides lower):

Upon further evaluation, we could merge config_local.toml and config_global.toml. WDYT?

jwmatthews commented 2 weeks ago

I updated how config priority is done. It's now done in the following way (higher overrides lower):

  • Config file that is declared on the command line / via init arguments. If using podman compose up, the config file in build/config.toml is supplied as a command line argument.
  • Environment variables

Is it possible to swap this and allow Environment variables to have top precedence?

When I think ofpodman compose up my ideal preference would be:

  1. Environment variables
  2. Settings in compose.yaml
  3. Configuration file (I'm assuming for this PR, build/config.toml would be the configuration file used.

Reason being is a user who is using podman compose up likely knows little about our project. I think they will be expecting that:

I assume in time they will understand we also have a configuration file they can view/edit at build/config.toml but I don't think it will be top of their mind on initial evaluation of Kai.

  • Local config file, kai/config_local.toml. This is for quick changes when running on bare metal.
  • Global config file, kai/config_default.toml. Sane defaults.
  • Default field values

Upon further evaluation, we could merge config_local.toml and config_global.toml. WDYT?

I didn't see a config_global.toml. but I will assume you are talking about merging config_local.toml and config_default.toml?

When I see config_local.toml I think of a local config change I'm running for my own testing and this would be handled outside of the git tracking. For example, today I will tweak our kai/config.toml for various models while testing, then when I'm committing my PR I'll revert my changes to kai/config.toml. I could see something like a config_local.toml as an override that takes higher precedence and allows us to specify a few settings for convenience without needing to edit/revert the config between various PRs.

I'm not advocating we want/need that, just sharing what went through my head when I saw this.

I don't see the config_local.toml in .gitignore not sure if you were intending it for the same thing that was in my mind or something else.

JonahSussman commented 2 weeks ago

Going to hold off on merging due to #332 preventing me from testing. It was working earlier but after deleting the logs directory, which was having permission errors on bare metal, I can no longer run podman compose up. Very very frustrating.

If anyone else can verify everything works then I'm good to merge.

jwmatthews commented 2 weeks ago

Closes https://github.com/konveyor/kai/issues/264

JonahSussman commented 2 weeks ago

Confirmed working on Ubuntu 24.04.1