Drop in config #379

ericcurtin · 2024-10-27T15:45:31Z

For setting configs in a config file

ericcurtin · 2024-10-27T15:46:09Z

@Conan-Kudo PTAL , haven't tested or anything yet

ericcurtin · 2024-10-27T16:01:43Z

Tests to fix

rhatdan · 2024-10-28T11:15:24Z

ramalama/cli.py

@@ -52,8 +52,63 @@ def add_argument(self, *args, help=None, default=None, **kwargs):
        super().add_argument(*args, **kwargs)


+def parse_config_file(file_path, config):


Lets move the config parsing to a separate file config.py?

Isn't there a standard toml parser written in Python that we can use?

import tomllib with open("/usr/share/containers/containers.conf", "rb") as f: data = tomllib.load(f) print(data)

tomllib is available since 3.11, not available on all platforms (some go back as far as 3.9).

Conan-Kudo · 2024-10-28T21:41:38Z

I'd recommend using tomlkit here if you want to use TOML for configuration.

ericcurtin · 2024-10-28T22:33:57Z

The thing is, if we write a simple toml parser like below, which is easy to maintain. We don't necessarily have to worry about whether somebody has that python library available on platform X:

import re

class TOMLParser:
    def __init__(self, toml_string=""):
        self.data = {}
        if toml_string:
            self.parse(toml_string)

    def parse(self, toml_string):
        current_section = self.data
        for line in toml_string.splitlines():
            line = line.strip()
            if not line or line.startswith("#"):
                continue

            if line.startswith("[") and line.endswith("]"):
                section_name = line[1:-1].strip()
                current_section = self._create_section(section_name)
            elif "=" in line:
                key, value = line.split("=", 1)
                key = key.strip()
                value = self._parse_value(value.strip())
                current_section[key] = value
            else:
                raise ValueError(f"Invalid TOML line: {line}")
        return self.data

    def _create_section(self, section_name):
        keys = section_name.split(".")
        section = self.data
        for key in keys:
            if key not in section:
                section[key] = {}
            section = section[key]
        return section

    def _parse_value(self, value):
        if value.startswith('"') and value.endswith('"'):
            return value[1:-1]
        elif value.startswith("[") and value.endswith("]"):
            return [self._parse_value(v.strip()) for v in value[1:-1].split(",")]
        elif re.match(r'^\d+$', value):
            return int(value)
        elif re.match(r'^\d+\.\d+$', value):
            return float(value)
        elif value.lower() in {"true", "false"}:
            return value.lower() == "true"
        else:
            raise ValueError(f"Unsupported value type: {value}")

    def get(self, key, default=None):
        keys = key.split(".")
        value = self.data
        for key in keys:
            value = value.get(key)
            if value is None:
                return default
        return value

ericcurtin · 2024-10-28T22:37:30Z

tomlkit is only in EPEL which isn't ideal. toml and tomli is in AppStream. But thinking outside of RHEL/Fedora I'd rather just write a small toml parser to cover macOS and other random Linux distros.

ericcurtin · 2024-10-29T11:55:38Z

README.md

@@ -15,7 +15,7 @@ Running in containers eliminates the need for users to configure the host system

 RamaLama then pulls AI Models from model registries. Starting a chatbot or a rest API service from a simple single command. Models are treated similarly to how Podman and Docker treat container images.

-When both Podman and Docker are installed, RamaLama defaults to Podman, The `RAMALAMA_CONTAINER_ENGINE=docker` environment variable can override this behavior. When neather are installed RamaLama will attempt to run the model with software on the local system.
+When both Podman and Docker are installed, RamaLama defaults to Podman, The `RAMALAMA_CONTAINER_ENGINE=docker` environment variable can override this behaviour. When neather are installed RamaLama will attempt to run the model with software on the local system.


Today I learned behavior vs behaviour is an US English vs UK English thing. The UK version for whatever reason is more prominent for this word in RamaLama. Can default to US English though if needs be.

rhatdan · 2024-10-29T14:24:43Z

Will your patches work on Windows?

We need to have multi-levels of defaults.

Unix Descriptions
            Built-in defaults
            Overwritten by:
                     /usr/share/ramamalam/containers.conf
            Overwritten by:
                     /etc/ramamalam/containers.conf
            Overwritten by:
                     /usr/share/ramamalam/containers.conf.d/*
            Overwritten by:
                     $HOME/.config/ramamalam/containers.conf
            Overwritten by:
                     Environment Variables
            Overwritten by command line options:
                    --engine=foobar

        Overwritten by:

ericcurtin · 2024-10-29T15:12:59Z

Will your patches work on Windows?

This will work using WSL2. There's a lot of things in RamaLama that won't work on Windows natively. For one, we use symlinks, which there isn't an exact equivalent for in Windows, although there are similar things.

We need to have multi-levels of defaults.

Unix Descriptions
            Built-in defaults
            Overwritten by:
                     /usr/share/ramamalam/containers.conf
            Overwritten by:
                     /etc/ramamalam/containers.conf
            Overwritten by:
                     /usr/share/ramamalam/containers.conf.d/*
            Overwritten by:
                     $HOME/.config/ramamalam/containers.conf
            Overwritten by:
                     Environment Variables
            Overwritten by command line options:
                    --engine=foobar

        Overwritten by:

SGTM

ericcurtin · 2024-10-29T15:25:44Z

Although. This seems funny to me:

            Overwritten by:
                     /usr/share/ramamalam/containers.conf
            Overwritten by:
                     /etc/ramamalam/containers.conf
            Overwritten by:
                     /usr/share/ramamala/containers.conf.d/*

I would expect this (/usr -> /etc -> $HOME):

            Overwritten by:
                     /usr/share/ramamala/containers.conf
            Overwritten by:
                     /usr/share/ramamala/containers.conf.d/*
            Overwritten by:
                     /etc/ramamala/containers.conf
            Overwritten by:
                     /etc/ramamala/containers.conf.d/*
            Overwritten by:
                     $HOME/.config/ramamala/containers.conf
            Overwritten by:
                     $HOME/.config/ramamala/containers.conf.d/*

rhatdan · 2024-10-29T17:37:30Z

We can leave out .d directories for now, probably overkill for a simple tool.

For setting configs in a config file Signed-off-by: Eric Curtin <[email protected]>

ericcurtin · 2024-10-29T19:50:01Z

We can leave out .d directories for now, probably overkill for a simple tool.

I can delete... But it's already implemented, it's just 3 lines of code in the current implementation... Easy to remove though, just delete three lines:

        if os.path.exists(path):
            # Load the main config file
            config = parser.parse_file(path)
        if os.path.isdir(path + ".d"):
            # Load all .conf files in ramalama.conf.d directory
            for conf_file in sorted(Path(path + ".d").glob("*.conf")):
                config = parser.parse_file(conf_file)

rhatdan · 2024-10-29T20:25:26Z

We can merge and continue to iterate. I would want a ramalama.conf file shipped in /usr/share/ramalama/ramalama.conf with all options commented out and explained.

Also needs ramalaman.5.md man page explaining how the configuration files work.

Finally we need tests to make sure things like ramalama --help shows the changed defaults.

Should have a RAMALAMA_CONFIG environment variable that forces the tooling to only read one config file for testing purposes.

ericcurtin force-pushed the drop-in-config branch 2 times, most recently from 469086b to adc34b2 Compare October 27, 2024 15:57

ericcurtin marked this pull request as draft October 27, 2024 16:01

rhatdan reviewed Oct 28, 2024

View reviewed changes

ericcurtin force-pushed the drop-in-config branch 4 times, most recently from f914dd5 to 1be6232 Compare October 29, 2024 11:52

ericcurtin commented Oct 29, 2024

View reviewed changes

ericcurtin force-pushed the drop-in-config branch 2 times, most recently from 3a24ae9 to f044858 Compare October 29, 2024 12:13

ericcurtin marked this pull request as ready for review October 29, 2024 12:46

This was referenced Oct 29, 2024

Docs: Typo Fix #387

Closed

Docs: Typo Fix #384

Closed

ericcurtin marked this pull request as draft October 29, 2024 14:03

ericcurtin force-pushed the drop-in-config branch from f044858 to c81dcab Compare October 29, 2024 17:59

Drop in config

0e6db50

For setting configs in a config file Signed-off-by: Eric Curtin <[email protected]>

ericcurtin force-pushed the drop-in-config branch from c81dcab to 0e6db50 Compare October 29, 2024 18:09

ericcurtin marked this pull request as ready for review October 29, 2024 18:55

rhatdan merged commit c2300fc into main Oct 29, 2024
12 checks passed

ericcurtin deleted the drop-in-config branch October 29, 2024 20:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop in config #379

Drop in config #379

ericcurtin commented Oct 27, 2024

ericcurtin commented Oct 27, 2024

ericcurtin commented Oct 27, 2024

rhatdan Oct 28, 2024

rhatdan Oct 28, 2024

ericcurtin Oct 28, 2024 •

edited

Loading

Conan-Kudo commented Oct 28, 2024

ericcurtin commented Oct 28, 2024

ericcurtin commented Oct 28, 2024

ericcurtin Oct 29, 2024 •

edited

Loading

rhatdan commented Oct 29, 2024

ericcurtin commented Oct 29, 2024

ericcurtin commented Oct 29, 2024 •

edited

Loading

rhatdan commented Oct 29, 2024

ericcurtin commented Oct 29, 2024

rhatdan commented Oct 29, 2024

		@@ -52,8 +52,63 @@ def add_argument(self, args, help=None, default=None, *kwargs):
		super().add_argument(args, *kwargs)


		def parse_config_file(file_path, config):

Drop in config #379

Drop in config #379

Conversation

ericcurtin commented Oct 27, 2024

ericcurtin commented Oct 27, 2024

ericcurtin commented Oct 27, 2024

rhatdan Oct 28, 2024

Choose a reason for hiding this comment

rhatdan Oct 28, 2024

Choose a reason for hiding this comment

ericcurtin Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

Conan-Kudo commented Oct 28, 2024

ericcurtin commented Oct 28, 2024

ericcurtin commented Oct 28, 2024

ericcurtin Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

rhatdan commented Oct 29, 2024

ericcurtin commented Oct 29, 2024

ericcurtin commented Oct 29, 2024 • edited Loading

rhatdan commented Oct 29, 2024

ericcurtin commented Oct 29, 2024

rhatdan commented Oct 29, 2024

ericcurtin Oct 28, 2024 •

edited

Loading

ericcurtin Oct 29, 2024 •

edited

Loading

ericcurtin commented Oct 29, 2024 •

edited

Loading