Repository: asphalt-framework/asphalt

Issue:
Configuration through the command line
Asphalt can currently be configured through YAML files, but it would be great to also support configuration through CLI arguments and options, that would take precedence over YAML files.

Discussion:
davidbrochart: @agronholm How do you feel about this PR, do you think it could go in?
agronholm: Yeah, in principle I like it. I've just been too busy trying to sort out the design issues with the component system. Some details will need adjustments. One of the issues I've discovered with the existing config system is that some keys need to actually have dots in them (such as logging configuration) and this current system does not work properly in such cases since `a.b.c: foo` becomes:
```yaml
a:
  b:
    c: foo
```
instead of
```yaml
a.b.c: foo
```
davidbrochart: Good point, I missed this case. What about using quotes to handle it? For instance, `--set "a.b.c"=foo`?
agronholm: How would you use that to set the logging level for `asphalt.templating` to `DEBUG`?
```yaml
logging:
  version: 1
  root:
    level: WARNING
  loggers:
    asphalt.templating:
      level: INFO
```
davidbrochart: Maybe `--set logging.loggers."asphalt.templating".level=DEBUG`? It looks a bit awkward, but I cannot find a case where it would not work.
agronholm: Except when the shell removes the quotes?
davidbrochart: Does it? Single quotes too?
agronholm: > Does it? Single quotes too?

Yup, both ways.
davidbrochart: The other thing I'm thinking about is an escape character: `--set logging.loggers.asphalt\.templating.level=DEBUG`.
agronholm: > The other thing I'm thinking about is an escape character: `--set logging.loggers.asphalt\.templating.level=DEBUG`.

Except those are _also_ removed by the shell. You'd have to insert it twice (`\\`) or quote the whole thing too.
agronholm: That said, dots in keys are rather rare, so just documenting this ought to be enough.
davidbrochart: > That said, dots in keys are rather rare, so just documenting this ought to be enough.

You mean [this solution](https://github.com/asphalt-framework/asphalt/pull/50#issuecomment-1328349875)?
agronholm: Yeah, the double backslash thing.
davidbrochart: Yes, I also think it's good enough.
agronholm: Ok, add that and I'll review this in more depth in a couple days.
davidbrochart: :+1: 
davidbrochart: CLI configuration is now converted to YAML, so that it goes through the same machinery as YAML file-based configuration. This fixes the casting issue. Also, CLI configuration is now generic, not limited to component configuration.
davidbrochart: Nice! That's better indeed.

Patch:
diff --git a/docs/userguide/deployment.rst b/docs/userguide/deployment.rst
index 6736615..7f9fd45 100644
--- a/docs/userguide/deployment.rst
+++ b/docs/userguide/deployment.rst
@@ -17,17 +17,18 @@ Running the launcher is very straightfoward:
 
 .. code-block:: bash
 
-    asphalt run yourconfig.yaml [your-overrides.yml...]
+    asphalt run yourconfig.yaml [your-overrides.yml...] [--set path.to.key=val]
 
 Or alternatively:
 
-    python -m asphalt run yourconfig.yaml [your-overrides.yml...]
+    python -m asphalt run yourconfig.yaml [your-overrides.yml...] [--set path.to.key=val]
 
 What this will do is:
 
 #. read all the given configuration files, starting from ``yourconfig.yaml``
-#. merge the configuration files' contents into a single configuration dictionary using
-   :func:`~asphalt.core.utils.merge_config`
+#. read the command line configuration options passed with ``--set``, if any
+#. merge the configuration files' contents and the command line configuration options into a single configuration dictionary using
+   :func:`~asphalt.core.utils.merge_config`.
 #. call :func:`~asphalt.core.runner.run_application` using the configuration dictionary as keyword
    arguments
 
@@ -147,8 +148,10 @@ Component configuration can be specified on several levels:
 * First configuration file argument to ``asphalt run``
 * Second configuration file argument to ``asphalt run``
 * ...
+* Command line configuration options to ``asphalt run --set``
 
 Any options you specify on each level override or augment any options given on previous levels.
+The command line configuration options have precedence over the configuration files.
 To minimize the effort required to build a working configuration file for your application, it is
 suggested that you pass as many of the options directly in the component initialization code and
 leave only deployment specific options like API keys, access credentials and such to the
@@ -162,12 +165,29 @@ gets passed three keyword arguments:
 * ``ssl=True``
 
 The first one is provided in the root component code while the other two options come from the YAML
-file. You could also override the mailer backend in the configuration file if you wanted. The same
-effect can be achieved programmatically by supplying the override configuration to the container
-component via its ``components`` constructor argument. This is very useful when writing tests
-against your application. For example, you might want to use the ``mock`` mailer in your test suite
-configuration to test that the application correctly sends out emails (and to prevent them from
-actually being sent to recipients!).
+file. You could also override the mailer backend in the configuration file if you wanted, or at the
+command line (with the configuration file saved as ``config.yaml``):
+
+.. code-block:: bash
+
+    asphalt run config.yaml --set component.components.mailer.backend=sendmail
+
+.. note::
+    Note that if you want a ``.`` to be treated as part of an identifier, and not as a separator,
+    you need to escape it at the command line with ``\``. For instance, in both commands:
+
+    .. code-block:: bash
+
+        asphalt run config.yaml --set "logging.loggers.asphalt\.templating.level=DEBUG"
+        asphalt run config.yaml --set logging.loggers.asphalt\\.templating.level=DEBUG
+
+    The logging level for the ``asphalt.templating`` logger will be set to ``DEBUG``.
+
+The same effect can be achieved programmatically by supplying the override configuration to the
+container component via its ``components`` constructor argument. This is very useful when writing
+tests against your application. For example, you might want to use the ``mock`` mailer in your test
+suite configuration to test that the application correctly sends out emails (and to prevent them
+from actually being sent to recipients!).
 
 There is another neat trick that lets you easily modify a specific key in the configuration.
 By using dotted notation in a configuration key, you can target a specific key arbitrarily deep in
diff --git a/src/asphalt/core/cli.py b/src/asphalt/core/cli.py
index 3f4c02d..b1823e5 100644
--- a/src/asphalt/core/cli.py
+++ b/src/asphalt/core/cli.py
@@ -1,8 +1,10 @@
 from __future__ import annotations
 
 import os
+import re
+from collections.abc import Mapping
 from pathlib import Path
-from typing import Any, Dict, Optional
+from typing import Any, Dict, List, Optional
 
 import click
 from ruamel.yaml import YAML, ScalarNode
@@ -52,7 +54,20 @@ def main() -> None:
     type=str,
     help="service to run (if the configuration file contains multiple services)",
 )
-def run(configfile, unsafe: bool, loop: Optional[str], service: Optional[str]) -> None:
+@click.option(
+    "--set",
+    "set_",
+    multiple=True,
+    type=str,
+    help="set configuration",
+)
+def run(
+    configfile,
+    unsafe: bool,
+    loop: Optional[str],
+    service: Optional[str],
+    set_: List[str],
+) -> None:
     yaml = YAML(typ="unsafe" if unsafe else "safe")
     yaml.constructor.add_constructor("!Env", env_constructor)
     yaml.constructor.add_constructor("!TextFile", text_file_constructor)
@@ -67,6 +82,28 @@ def run(configfile, unsafe: bool, loop: Optional[str], service: Optional[str]) -
         ), "the document root element must be a dictionary"
         config = merge_config(config, config_data)
 
+    # Override config options
+    for override in set_:
+        if "=" not in override:
+            raise click.ClickException(
+                f"Configuration must be set with '=', got: {override}"
+            )
+
+        key, value = override.split("=", 1)
+        parsed_value = yaml.load(value)
+        keys = [k.replace(r"\.", ".") for k in re.split(r"(?<!\\)\.", key)]
+        section = config
+        for i, part_key in enumerate(keys[:-1]):
+            section = section.setdefault(part_key, {})
+            if not isinstance(section, Mapping):
+                path = " ⟶ ".join(x for x in keys[: i + 1])
+                raise click.ClickException(
+                    f"Cannot apply override for {key!r}: value at {path} is not "
+                    f"a mapping, but {qualified_name(section)}"
+                )
+
+        section[keys[-1]] = parsed_value
+
     # Override the event loop policy if specified
     if loop:
         config["event_loop_policy"] = loop


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index c8c22f6..02247a9 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -88,6 +88,38 @@ logging:
         }
 
 
+def test_run_bad_override(runner: CliRunner) -> None:
+    config = """\
+    component:
+        type: does.not.exist:Component
+"""
+    with runner.isolated_filesystem():
+        Path("test.yml").write_text(config)
+        result = runner.invoke(cli.run, ["test.yml", "--set", "foobar"])
+        assert result.exit_code == 1
+        assert result.stdout == (
+            "Error: Configuration must be set with '=', got: foobar\n"
+        )
+
+
+def test_run_bad_path(runner: CliRunner) -> None:
+    config = """\
+    component:
+        type: does.not.exist:Component
+        listvalue: []
+"""
+    with runner.isolated_filesystem():
+        Path("test.yml").write_text(config)
+        result = runner.invoke(
+            cli.run, ["test.yml", "--set", "component.listvalue.foo=1"]
+        )
+        assert result.exit_code == 1
+        assert result.stdout == (
+            "Error: Cannot apply override for 'component.listvalue.foo': value at "
+            "component ⟶ listvalue is not a mapping, but list\n"
+        )
+
+
 def test_run_multiple_configs(runner: CliRunner) -> None:
     component_class = "{0.__module__}:{0.__name__}".format(DummyComponent)
     config1 = """\
@@ -106,6 +138,7 @@ logging:
 component:
   dummyval1: alternate
   dummyval2: 10
+  dummyval3: foo
 """
 
     with runner.isolated_filesystem(), patch(
@@ -113,7 +146,17 @@ component:
     ) as run_app:
         Path("conf1.yml").write_text(config1)
         Path("conf2.yml").write_text(config2)
-        result = runner.invoke(cli.run, ["conf1.yml", "conf2.yml"])
+        result = runner.invoke(
+            cli.run,
+            [
+                "conf1.yml",
+                "conf2.yml",
+                "--set",
+                "component.dummyval3=bar",
+                "--set",
+                "component.dummyval4=baz",
+            ],
+        )
 
         assert result.exit_code == 0
         assert run_app.call_count == 1
@@ -124,6 +167,8 @@ component:
                 "type": component_class,
                 "dummyval1": "alternate",
                 "dummyval2": 10,
+                "dummyval3": "bar",
+                "dummyval4": "baz",
             },
             "logging": {"version": 1, "disable_existing_loggers": False},
         }