Environment variables silently fail to override YAML config values

[craigmcchesney]

Environment variables do not override values set in the YAML configuration file. This inverts levels 2 and 3 of the documented configuration priority, and it fails silently — no warning, no error.

Found while writing the configuration recipe for #15.

Behavior

The rule is per-key: a key absent from the YAML file is overridable by its MLDP_* env var; a key present in the file is not.

Given a mldp-config.yaml containing only ingestion: {host: localhost}:

Env var	Present in YAML?	Resolves to	Correct?
MLDP_INGESTION_HOST=prod.example.com	yes	localhost	❌ env ignored
MLDP_INGESTION_PORT=443	no	443	✅

Reproduction

mldp-config.yaml:

ingestion:
  host: localhost

import os
from dp_python_lib.config import MldpConfig

cfg = MldpConfig.from_yaml("mldp-config.yaml")
print(cfg.ingestion.host)   # 'localhost'  <- MLDP_INGESTION_HOST ignored
print(cfg.ingestion.port)   # 443          <- MLDP_INGESTION_PORT applied

MLDP_INGESTION_HOST=prod.example.com MLDP_INGESTION_PORT=443 python repro.py

Reproduced on dp-python-lib 1.15.0 with pydantic-settings 2.14.2.

Cause

MldpConfig.from_yaml() (src/dp_python_lib/config/config.py:82-109) flattens the YAML into flat_data and passes it as constructor keyword arguments:

return cls(**flat_data)          # config.py:109

In pydantic-settings, init kwargs are the highest-priority source — above environment variables. So every key the YAML file happens to define becomes un-overridable.

This is on the path MldpClient() actually takes: load_config() (loader.py:105) calls from_yaml() whenever a config file is discovered, and mldp-config.yaml in the working directory is discovered automatically.

Why it wasn't caught

tests/unit/test_config.py::test_environment_variable_override (line 87) constructs MldpConfig() directly, where env vars are the highest-priority source. It passes, and correctly so.

No test covers from_yaml() or load_config() with a YAML file present — which is the real-world path. The bug sits exactly in that gap.

Impact

Breaks the deployment pattern env vars exist for: one built image, per-environment overrides. A deployment that ships a mldp-config.yaml and expects MLDP_* to retarget it will silently connect to the wrong host.

The silence is the worst part — the client connects successfully, just not where intended.

Suggested fix

Route YAML through a settings source ranked below env vars, rather than through init kwargs, via settings_customise_sources:

@classmethod
def settings_customise_sources(cls, settings_cls, init_settings, env_settings,
                               dotenv_settings, file_secret_settings):
    # documented priority: init > env > yaml > defaults
    return (init_settings, env_settings, YamlSource(settings_cls), file_secret_settings)

I verified this approach restores the documented order on the current pydantic-settings version:

• key in YAML + env var set → env wins ✅
• key in YAML, no env var → YAML wins (over the built-in default) ✅
• key absent from YAML → default, or env var if set ✅

Whatever the fix, it should come with tests covering load_config() with a YAML file present, since that is the untested path where this hid.

Docs

Documented as a known bug with a ⚠️ callout in doc/cookbook/connecting.md (#15), including the verified workaround: keep the setting out of the YAML, or repoint the whole file with MLDP_CONFIG_FILE (that variable is read before the file is loaded, so it behaves as documented).

Remove that callout when this is fixed. README.md and CLAUDE.md both currently document the intended priority as though it works.

[craigmcchesney]

Triage — confirmed, deferred to the first post-release cycle

Reproduced independently on the current tree (pydantic-settings 2.14.2). The report and its diagnosis are accurate; cls(**flat_data) at config.py:109 makes every YAML-defined key un-overridable, because init kwargs outrank env vars in pydantic-settings.

# mldp-config.yaml contains only:  ingestion: {host: localhost}
MLDP_INGESTION_HOST=prod.example.com MLDP_INGESTION_PORT=443 python repro.py
->  host: localhost   port: 443

Disposition: worth fixing, not urgent, scheduled after the release. One customer, in evaluation, almost certainly single-environment — which is exactly the case this bug cannot reach, since it only manifests in the build-one-image-override-per-environment pattern. The cookbook (#15) already documents it with a ⚠️ callout and a verified workaround. A documented bug with a working workaround is not a release blocker, and this sits on the code path every MldpClient() construction takes — not where we want a same-day change.

Detail gathered during triage that shouldn't have to be rediscovered:

The suggested fix works — verified

I prototyped the settings_customise_sources approach against pydantic-settings 2.14.2 and confirmed it restores the documented order:

Case	Result
key in YAML + env var set	env wins ✅
key in YAML, no env var	YAML wins over default ✅
key absent from YAML	default, or env var if set ✅

Open decision: load_config(config_object=...) has the same mechanism

loader.py:87-97 re-passes an explicit config object as init kwargs, so it beats env vars by the same route as the bug. Verified:

load_config(config_object=MldpConfig(ingestion_host="explicit-host"))
with MLDP_INGESTION_HOST=prod.example.com  ->  "explicit-host"

Here that outcome is correct per the documented priority — explicit parameters are level 1, above env vars. But the natural fix touches the shared constructor path, so this call site's behavior could shift as a side effect. It should stay as-is; the fix needs to preserve it deliberately, with a test pinning it.

Worth noting the current code disagrees with itself about this: loader.py:85 logs "Using explicit config object with environment variable overrides" and loader.py:86 comments "Create a new instance that will pick up environment variables" — neither of which is what the code does, or (in my reading) what it should do. The log line and comment should be corrected to match the documented level-1 semantics.

Test gap is wider than "no coverage of the YAML path"

test_config.py:87 (test_environment_variable_override) constructs MldpConfig() directly, where env vars genuinely are highest-priority. It passes, correctly, and would keep passing no matter how broken from_yaml() got.

The sharper problem is test_load_config_from_yaml (test_config.py:234): it @patches MldpConfig.from_yaml outright, so it asserts only that the loader calls the right function with the right filename — never what that function resolves to. The one test aimed at the real path is mocked precisely at the layer where the bug lives.

So the fix needs tests that write a real YAML file to a tmpdir and drive load_config() end to end with MLDP_* set, covering the full matrix above plus the config_object precedence case. Mock-based tests cannot catch a regression here.

Docs to update when this lands

• doc/cookbook/connecting.md — remove the ⚠️ callout under "Configuration priority" (~line 158) and the "and a bug to be aware of" pointer in the ToC (line 16).
• README.md and CLAUDE.md need no change — both already document the intended priority, which is what the fix delivers.

Estimate

Roughly half a day. The settings-source swap is small and already prototyped; most of the time is the real-YAML test layer and un-mocking test_load_config_from_yaml.