Repository: Materials-Consortia/optimade-python-tools

Issue:
Custom configuration extensions & use standard pydantic way of loading config file
It would be nice to be able to extend the configuration class with custom configuration options, but this is currently not allowed (`extra` attribute in embedded `Config` class).

Furthermore, [pydantic](https://pydantic-docs.helpmanual.io/usage/settings/#adding-sources) seems to have a more "standard" way of loading a config file than what we're doing. It might be worth testing out.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/Materials-Consortia/optimade-python-tools/pull/740?src=pr&el=h1) Report
> Merging [#740](https://codecov.io/gh/Materials-Consortia/optimade-python-tools/pull/740?src=pr&el=desc) (af63ea0) into [master](https://codecov.io/gh/Materials-Consortia/optimade-python-tools/commit/9306cb835b17dc21729a1f39df289f3d33e97912?el=desc) (9306cb8) will **increase** coverage by `0.01%`.
> The diff coverage is `76.92%`.

[![Impacted file tree graph](https://codecov.io/gh/Materials-Consortia/optimade-python-tools/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=UJAtmqkZZO)](https://codecov.io/gh/Materials-Consortia/optimade-python-tools/pull/740?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #740      +/-   ##
==========================================
+ Coverage   92.78%   92.79%   +0.01%     
==========================================
  Files          67       67              
  Lines        3533     3539       +6     
==========================================
+ Hits         3278     3284       +6     
  Misses        255      255              
```

| Flag | Coverage Δ | |
|---|---|---|
| project | `92.79% <76.92%> (+0.01%)` | :arrow_up: |
| validator | `63.69% <76.92%> (+0.06%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/Materials-Consortia/optimade-python-tools/pull/740?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [optimade/server/config.py](https://codecov.io/gh/Materials-Consortia/optimade-python-tools/pull/740/diff?src=pr&el=tree#diff-b3B0aW1hZGUvc2VydmVyL2NvbmZpZy5weQ==) | `89.18% <76.92%> (+0.95%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Materials-Consortia/optimade-python-tools/pull/740?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Materials-Consortia/optimade-python-tools/pull/740?src=pr&el=footer). Last update [9306cb8...af63ea0](https://codecov.io/gh/Materials-Consortia/optimade-python-tools/pull/740?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

CasperWA: > To avoid trying to hit a moving target, can we consider this PR blocked by #660? There may be some CI config added there that will need to be updated given these changes.

Suuuure ... :)
Let's get #660 in then.
CasperWA: It seems we're running into an issue coming from `sqlalchemy` having just released v1.4.0. I've opened an issue to follow this at aiidateam/aiida-core#4808.

Patch:
diff --git a/.github/workflows/deps_lint.yml b/.github/workflows/deps_lint.yml
index ade14fc5..1f789fc0 100644
--- a/.github/workflows/deps_lint.yml
+++ b/.github/workflows/deps_lint.yml
@@ -204,6 +204,9 @@ jobs:
       run: |
         pip install -r requirements-client.txt
         # AiiDA-specific
+        # sqlalchemy install is temporary and has been fixed in aiidateam/aiida-core#4809,
+        # but is still not released in a new AiiDA version.
+        pip install sqlalchemy==1.3.23
         reentry scan
 
     - name: Setup up environment for AiiDA
diff --git a/optimade/server/config.py b/optimade/server/config.py
index f77a91c1..ffc61dd9 100644
--- a/optimade/server/config.py
+++ b/optimade/server/config.py
@@ -1,32 +1,52 @@
 # pylint: disable=no-self-argument
 from enum import Enum
-import json
+from pathlib import Path
 import warnings
-from typing import Optional, Dict, List
+from typing import Any, Dict, List, Optional, Tuple
 
 try:
     from typing import Literal
 except ImportError:
     from typing_extensions import Literal
-from pathlib import Path
 
 from pydantic import (  # pylint: disable=no-name-in-module
+    AnyHttpUrl,
     BaseSettings,
     Field,
     root_validator,
-    AnyHttpUrl,
     validator,
 )
+from pydantic.env_settings import SettingsSourceCallable
 
 from optimade import __version__
 from optimade.models import Implementation, Provider
 
 
-DEFAULT_CONFIG_FILE_PATH = str(Path.home().joinpath(".optimade.json"))
+DEFAULT_CONFIG_FILE_PATH: str = str(Path.home().joinpath(".optimade.json"))
+"""Default configuration file path.
+
+This variable is used as the fallback value if the environment variable `OPTIMADE_CONFIG_FILE` is
+not set.
+
+!!! note
+    It is set to: `pathlib.Path.home()/.optimade.json`
+
+    For Unix-based systems (Linux) this will be equivalent to `~/.optimade.json`.
+
+"""
 
 
 class LogLevel(Enum):
-    """Replication of logging LogLevels"""
+    """Replication of logging LogLevels
+
+    - `notset`
+    - `debug`
+    - `info`
+    - `warning`
+    - `error`
+    - `critical`
+
+    """
 
     NOTSET = "notset"
     DEBUG = "debug"
@@ -37,21 +57,84 @@ class LogLevel(Enum):
 
 
 class SupportedBackend(Enum):
+    """Enumeration of supported database backends
+
+    - `elastic`: [Elasticsearch](https://www.elastic.co/).
+    - `mongodb`: [MongoDB](https://www.mongodb.com/).
+    - `mongomock`: Also MongoDB, but instead of using the
+        [`pymongo`](https://pymongo.readthedocs.io/) driver to connect to a live Mongo database
+        instance, this will use the [`mongomock`](https://github.com/mongomock/mongomock) driver,
+        creating an in-memory database, which is mainly used for testing.
+
+    """
 
     ELASTIC = "elastic"
     MONGODB = "mongodb"
     MONGOMOCK = "mongomock"
 
 
+def config_file_settings(settings: BaseSettings) -> Dict[str, Any]:
+    """Configuration file settings source.
+
+    Based on the example in the
+    [pydantic documentation](https://pydantic-docs.helpmanual.io/usage/settings/#adding-sources),
+    this function loads ServerConfig settings from a configuration file.
+
+    The file must be of either type JSON or YML/YAML.
+
+    Parameters:
+        settings: The `pydantic.BaseSettings` class using this function as a
+            `pydantic.SettingsSourceCallable`.
+
+    Returns:
+        Dictionary of settings as read from a file.
+
+    """
+    import json
+    import os
+    import yaml
+
+    encoding = settings.__config__.env_file_encoding
+    config_file = Path(os.getenv("OPTIMADE_CONFIG_FILE", DEFAULT_CONFIG_FILE_PATH))
+
+    res = {}
+    if config_file.is_file():
+        config_file_content = config_file.read_text(encoding=encoding)
+
+        try:
+            res = json.loads(config_file_content)
+        except json.JSONDecodeError as json_exc:
+            try:
+                # This can essentially also load JSON files, as JSON is a subset of YAML v1,
+                # but I suspect it is not as rigorous
+                res = yaml.safe_load(config_file_content)
+            except yaml.YAMLError as yaml_exc:
+                warnings.warn(
+                    f"Unable to parse config file {config_file} as JSON or YAML, using the "
+                    "default settings instead..\n"
+                    f"Errors:\n  JSON:\n{json_exc}.\n\n  YAML:\n{yaml_exc}"
+                )
+    else:
+        warnings.warn(
+            f"Unable to find config file at {config_file}, using the default settings instead."
+        )
+
+    if res is None:
+        # This can happen if the yaml loading doesn't succeed properly, e.g., if the file is empty.
+        warnings.warn(
+            "Unable to load any settings from {config_file}, using the default settings instead."
+        )
+        res = {}
+
+    return res
+
+
 class ServerConfig(BaseSettings):
     """This class stores server config parameters in a way that
     can be easily extended for new config file types.
 
     """
 
-    config_file: Optional[str] = Field(
-        None, description="File to load alternative defaults from"
-    )
     debug: bool = Field(
         False,
         description="Turns on Debug Mode for the OPTIMADE Server implementation",
@@ -59,7 +142,11 @@ class ServerConfig(BaseSettings):
 
     insert_test_data: bool = Field(
         True,
-        description="Insert test data into each collection on server initialisation. If true, the configured backend will be populated with test data on server start. Should be disabled for production usage.",
+        description=(
+            "Insert test data into each collection on server initialisation. If true, the "
+            "configured backend will be populated with test data on server start. Should be "
+            "disabled for production usage."
+        ),
     )
 
     use_real_mongo: Optional[bool] = Field(
@@ -96,14 +183,18 @@ class ServerConfig(BaseSettings):
     )
     default_db: str = Field(
         "test_server",
-        description="ID of /links endpoint resource for the chosen default OPTIMADE implementation (only relevant for the index meta-database)",
+        description=(
+            "ID of /links endpoint resource for the chosen default OPTIMADE implementation (only "
+            "relevant for the index meta-database)"
+        ),
     )
     root_path: Optional[str] = Field(
         None,
         description=(
-            "Sets the FastAPI app `root_path` parameter. This can be used to serve the API under a "
-            "path prefix behind a proxy or as a sub-application of another FastAPI app. "
-            "See https://fastapi.tiangolo.com/advanced/sub-applications/#technical-details-root_path for details."
+            "Sets the FastAPI app `root_path` parameter. This can be used to serve the API under a"
+            " path prefix behind a proxy or as a sub-application of another FastAPI app. See "
+            "https://fastapi.tiangolo.com/advanced/sub-applications/#technical-details-root_path "
+            "for details."
         ),
     )
     base_url: Optional[str] = Field(
@@ -135,25 +226,37 @@ class ServerConfig(BaseSettings):
         Literal["links", "references", "structures"], List[str]
     ] = Field(
         {},
-        description="A list of additional fields to be served with the provider's prefix attached, broken down by endpoint.",
+        description=(
+            "A list of additional fields to be served with the provider's prefix attached, "
+            "broken down by endpoint."
+        ),
     )
     aliases: Dict[Literal["links", "references", "structures"], Dict[str, str]] = Field(
         {},
-        description="A mapping between field names in the database with their corresponding OPTIMADE field names, broken down by endpoint.",
+        description=(
+            "A mapping between field names in the database with their corresponding OPTIMADE field"
+            " names, broken down by endpoint."
+        ),
     )
     length_aliases: Dict[
         Literal["links", "references", "structures"], Dict[str, str]
     ] = Field(
         {},
         description=(
-            "A mapping between a list property (or otherwise) and an integer property that defines the length of that list, "
-            "for example elements -> nelements. The standard aliases are applied first, so this dictionary must refer to the "
-            "API fields, not the database fields."
+            "A mapping between a list property (or otherwise) and an integer property that defines"
+            " the length of that list, for example elements -> nelements. The standard aliases are"
+            " applied first, so this dictionary must refer to the API fields, not the database "
+            "fields."
         ),
     )
     index_links_path: Path = Field(
         Path(__file__).parent.joinpath("index_links.json"),
-        description="Absolute path to a JSON file containing the MongoDB collection of /links resources for the index meta-database",
+        description=(
+            "Absolute path to a JSON file containing the MongoDB collecton of links entries "
+            "(documents) to serve under the /links endpoint of the index meta-database. "
+            "NB! As suggested in the previous sentence, these will only be served when using a "
+            "MongoDB-based backend."
+        ),
     )
     log_level: LogLevel = Field(
         LogLevel.INFO, description="Logging level for the OPTIMADE server."
@@ -179,7 +282,8 @@ class ServerConfig(BaseSettings):
         use_real_mongo = values.pop("use_real_mongo", None)
         if use_real_mongo is not None:
             warnings.warn(
-                "'use_real_mongo' is deprecated, please set the appropriate 'database_backend' instead.",
+                "'use_real_mongo' is deprecated, please set the appropriate 'database_backend' "
+                "instead.",
                 DeprecationWarning,
             )
 
@@ -188,44 +292,6 @@ class ServerConfig(BaseSettings):
 
         return values
 
-    @root_validator(pre=True)
-    def load_settings(cls, values):
-        """
-        Loads settings from a JSON config file, if available, and uses them in place
-        of the built-in defaults.
-        """
-        config_file_path = Path(values.get("config_file", DEFAULT_CONFIG_FILE_PATH))
-
-        new_values = {}
-
-        if config_file_path.is_file():
-            try:
-                with open(config_file_path) as f:
-                    new_values = json.load(f)
-            except json.JSONDecodeError as exc:
-                warnings.warn(
-                    f"Unable to parse config file {config_file_path} as JSON. Error: {exc}."
-                )
-
-        else:
-            if DEFAULT_CONFIG_FILE_PATH != str(config_file_path):
-                warnings.warn(
-                    f"Unable to find config file in requested location {config_file_path}, "
-                    "using the built-in default settings instead."
-                )
-            else:
-                warnings.warn(
-                    f"Unable to find config file in default location {DEFAULT_CONFIG_FILE_PATH}, "
-                    "using the built-in default settings instead."
-                )
-
-        if not new_values:
-            values["config_file"] = None
-
-        new_values.update(values)
-
-        return new_values
-
     class Config:
         """
         This is a pydantic model Config object that modifies the behaviour of
@@ -236,6 +302,39 @@ class ServerConfig(BaseSettings):
         """
 
         env_prefix = "optimade_"
+        extra = "allow"
+        env_file_encoding = "utf-8"
+
+        @classmethod
+        def customise_sources(
+            cls,
+            init_settings: SettingsSourceCallable,
+            env_settings: SettingsSourceCallable,
+            file_secret_settings: SettingsSourceCallable,
+        ) -> Tuple[SettingsSourceCallable, ...]:
+            """
+            **Priority of config settings sources**:
+
+            1. Passed arguments upon initialization of
+               [`ServerConfig`][optimade.server.config.ServerConfig].
+            2. Environment variables, matching the syntax: `"OPTIMADE_"` or `"optimade_"` +
+               `<config_name>`, e.g., `OPTIMADE_LOG_LEVEL=debug` or
+               `optimade_log_dir=~/logs_dir/optimade/`.
+            3. Configuration file (JSON/YAML) taken from:
+               1. Environment variable `OPTIMADE_CONFIG_FILE`.
+               2. Default location (see
+                  [DEFAULT_CONFIG_FILE_PATH][optimade.server.config.DEFAULT_CONFIG_FILE_PATH]).
+            4. Settings from secret file (see
+               [pydantic documentation](https://pydantic-docs.helpmanual.io/usage/settings/#secret-support)
+               for more information).
+
+            """
+            return (
+                init_settings,
+                env_settings,
+                config_file_settings,
+                file_secret_settings,
+            )
 
 
 CONFIG = ServerConfig()
diff --git a/optimade/server/main.py b/optimade/server/main.py
index 4cd745f1..04e11b44 100644
--- a/optimade/server/main.py
+++ b/optimade/server/main.py
@@ -5,7 +5,7 @@ The server is based on MongoDB, using either `pymongo` or `mongomock`.
 This is an example implementation with example data.
 To implement your own server see the documentation at https://optimade.org/optimade-python-tools.
 """
-
+import os
 import warnings
 
 from fastapi import FastAPI
@@ -33,13 +33,13 @@ from optimade.server.routers import (
 from optimade.server.routers.utils import BASE_URL_PREFIXES
 
 
-if CONFIG.config_file is None:
+if os.getenv("OPTIMADE_CONFIG_FILE") is None:
     LOGGER.warn(
         f"Invalid config file or no config file provided, running server with default settings. Errors: "
         f"{[warnings.formatwarning(w.message, w.category, w.filename, w.lineno, '') for w in config_warnings]}"
     )
 else:
-    LOGGER.info(f"Loaded settings from {CONFIG.config_file}.")
+    LOGGER.info(f"Loaded settings from {os.getenv('OPTIMADE_CONFIG_FILE')}.")
 
 if CONFIG.debug:  # pragma: no cover
     LOGGER.info("DEBUG MODE")
diff --git a/optimade/server/main_index.py b/optimade/server/main_index.py
index 7e00107b..6ff22d02 100644
--- a/optimade/server/main_index.py
+++ b/optimade/server/main_index.py
@@ -6,6 +6,7 @@ This is an example implementation with example data.
 To implement your own index meta-database server see the documentation at https://optimade.org/optimade-python-tools.
 """
 import json
+import os
 import warnings
 
 from fastapi import FastAPI
@@ -23,13 +24,13 @@ from optimade.server.middleware import OPTIMADE_MIDDLEWARE
 from optimade.server.routers import index_info, links, versions
 from optimade.server.routers.utils import BASE_URL_PREFIXES
 
-if CONFIG.config_file is None:
+if os.getenv("OPTIMADE_CONFIG_FILE") is None:
     LOGGER.warn(
         f"Invalid config file or no config file provided, running server with default settings. Errors: "
         f"{[warnings.formatwarning(w.message, w.category, w.filename, w.lineno, '') for w in config_warnings]}"
     )
 else:
-    LOGGER.info(f"Loaded settings from {CONFIG.config_file}.")
+    LOGGER.info(f"Loaded settings from {os.getenv('OPTIMADE_CONFIG_FILE')}.")
 
 if CONFIG.debug:  # pragma: no cover
     LOGGER.info("DEBUG MODE")
diff --git a/requirements.txt b/requirements.txt
index 7d583733..be1bf4e3 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -10,3 +10,4 @@ django==3.1.7
 elasticsearch-dsl==6.4.0
 Jinja2==2.11.3
 typing-extensions==3.7.4.3
+pyyaml==5.1.2
diff --git a/setup.py b/setup.py
index 2ccd83ce..b917c5c7 100644
--- a/setup.py
+++ b/setup.py
@@ -20,7 +20,7 @@ with open(module_dir.joinpath("optimade/__init__.py")) as version_file:
 django_deps = ["django>=2.2.9,<4.0"]
 elastic_deps = ["elasticsearch-dsl~=6.4,<7.0"]
 mongo_deps = ["pymongo~=3.11", "mongomock~=3.22"]
-server_deps = ["uvicorn~=0.13.4", "Jinja2~=2.11"] + mongo_deps
+server_deps = ["uvicorn~=0.13.4", "Jinja2~=2.11", "pyyaml~=5.1"] + mongo_deps
 
 # Client minded
 aiida_deps = ["aiida-core~=1.5.2"]


Test patch:
diff --git a/tests/server/test_config.py b/tests/server/test_config.py
index f780b1f7..86990f1f 100644
--- a/tests/server/test_config.py
+++ b/tests/server/test_config.py
@@ -127,3 +127,71 @@ def test_debug_is_respected_when_on(both_clients):
         assert f"_{CONFIG.provider.prefix}_traceback" in response["meta"]
     finally:
         CONFIG.debug = org_value
+
+
+def test_yaml_config_file():
+    """Test loading a YAML config file
+
+    First, pass a correctly formatted YAML file that only includes a single YAML document.
+    Secondly, pass a correctly formatted YAML file that includes two YAML docuements.
+
+    Expectation: The second test should fail to load with a warning, loading the default settings
+    instead. The first test should be fine.
+
+    NOTE: Since pytest loads a JSON config file, there's no need to test that further.
+    """
+    import tempfile
+    import pytest
+    from optimade.server.config import ServerConfig
+
+    yaml_content = """
+---
+aliases:
+  references:
+    last_modified: mtime
+debug: true
+provider_fields:
+  references:
+  - great_paper
+...
+"""
+    extra_yaml_content = """
+---
+debug: true
+...
+"""
+    org_env_var = os.getenv("OPTIMADE_CONFIG_FILE")
+
+    try:
+        with tempfile.NamedTemporaryFile("w+t", suffix=".yml") as config_file:
+            config_file.write(yaml_content)
+            config_file.seek(0, 0)  # Go to start of file
+
+            os.environ["OPTIMADE_CONFIG_FILE"] = str(Path(config_file.name).resolve())
+            CONFIG = ServerConfig()
+
+        assert CONFIG.aliases == {
+            "references": {"last_modified": "mtime"}
+        }, f"Config: {CONFIG.aliases}"
+        assert CONFIG.debug, f"Config: {CONFIG.debug}"
+        assert CONFIG.provider_fields == {
+            "references": ["great_paper"]
+        }, f"Config: {CONFIG.provider_fields}"
+
+        with tempfile.NamedTemporaryFile("w+t", suffix=".yml") as config_file:
+            config_file.write(yaml_content + extra_yaml_content)
+            config_file.seek(0, 0)  # Go to start of file
+
+            os.environ["OPTIMADE_CONFIG_FILE"] = str(Path(config_file.name).resolve())
+            with pytest.warns(UserWarning, match=r"^Unable to parse config file.*"):
+                CONFIG = ServerConfig()
+
+        assert CONFIG.aliases == {}, f"Config: {CONFIG.aliases}"
+        assert not CONFIG.debug, f"Config: {CONFIG.debug}"
+        assert CONFIG.provider_fields == {}, f"Config: {CONFIG.provider_fields}"
+
+    finally:
+        if org_env_var is not None:
+            os.environ["OPTIMADE_CONFIG_FILE"] = org_env_var
+        else:
+            assert os.getenv("OPTIMADE_CONFIG_FILE") is None