feat(configuration): stacked environment configuration overlays#54
feat(configuration): stacked environment configuration overlays#54eaguad1337 wants to merge 1 commit into
Conversation
Configuration can now be "stacked": an optional `overlays` argument to
`Configuration.load()` merges additional locations on top of the base config,
and an overlay at `{config.location}/environment/{APP_ENV}` is applied
automatically when that directory exists. This lets an environment declare only
the values it changes instead of pushing every customization through env vars.
Overlays are partial: dict values are deep-merged (override wins, siblings
preserved) while lists and scalars are replaced. A new pure `deep_merge` helper
implements the merge and never mutates its inputs.
Fully backwards compatible:
- `load()` with no arguments and no `environment/` folder behaves exactly as
before (verified against the existing suite);
- the base load still uses `raise_exception=True`, so a broken config module
fails loudly rather than being silently skipped;
- `merge_with` and the stored config representation are unchanged.
|
@eaguad1337 issue 1:
issue 2:
It just makes more sense to have the overlays as an entirely "opt in" mechanism. What do you think? |
| - lists, scalars and mismatched types are replaced by ``override``. | ||
|
|
There was a problem hiding this comment.
see my omment about replacing lists
| # automatically stack the overlay for the current environment when present. | ||
| environment = self.application.environment() | ||
| if environment: | ||
| environment_overlay = f"{config_root}/environment/{environment}" | ||
| if os.path.isdir(as_filepath(environment_overlay)): | ||
| self._apply_overlay(environment_overlay) | ||
|
|
There was a problem hiding this comment.
See my comment about environment naming problems.
|
Thanks for the review @circulon! Quick thoughts on each: 1. Lists: append instead of replace I'd skip the case-insensitive match for de-dup, it'd treat MyProvider and myprovider as the same, which feels bug-prone. Exact match is safer. 2. Auto-discovery by env name But I agree the implicit loading can be surprising. The opt-in path you want already exists via Happy to gate it behind a flag or just document it better instead of dropping it. |
Yeah i see yout point and having a choice sounds optimal with replace as the default. configuration = Configuration(self.application)
configuration.load([f"config.environment.core"])
configuration.load(["config.environment.prod", "config.environment.deployed"], replace=False)which would:
One thing I found in my considerations of this merging scenario was the "merge_deep" package
Yeah true ignore this
Aaah ok I see.
Yeah I see what your saying and if the env var Sating this feature as an "opt-out" on the Documenting with examples and the fact that this auto load functionality is specifically linked to the env var is the way to go I think One thing I would change is to put the "environment" auto load BEFORE any overlays are added as these are opt-in and would currently be changed/replaced by the config in the auto-loaded environment folder |
|
@eaguad1337 |
Summary
Adds backwards-compatible stacked / environment configuration: a base config plus thin per-environment overlays that only declare the values they change. This is an alternative to pushing every per-environment customization through
.envvariables.Configuration.load(overlays=None)— an optional list of overlay locations, merged in order on top of the base config.{config.location}/environment/{APP_ENV}is applied automatically when that directory exists (a no-op otherwise), so the feature works out of the box.deep_mergehelper implements this and never mutates its inputs.Relationship to #49
This supersedes #49, which proposed the same capability but could not be merged:
Application()inside a plainunittest.TestCase. BecauseContainer.objectsis a class variable, that clobbered shared global state and cascaded ~15 failures across unrelated suites (RouteMiddlewareNotFound: 'web').raise_exception=False, silently swallowing broken config modules for every user.SyntaxError, and asserted brittle magic key-counts.How this PR addresses each point
wsgiapplication via Masonite'sTestCaseand only swap the boundconfig.location(restored intearDown); a throwaway localConfigurationis asserted on, so the globally boundconfigis never touched.raise_exception=True, and overlays use it too — a broken config/overlay module fails loudly. Missing overlay modules are naturally skipped, so partial overlays still work.load(), so every call site gets it for free. New unit tests coverdeep_merge; integration-style tests cover explicit overlays, partial overlays, list replacement, automatic environment application, and the no-op case.Backwards compatibility
Fully backwards compatible — no major version bump:
load()with no arguments → unchanged behavior.config/environment/folder and no explicit overlays,load()behaves byte-for-byte as before.merge_withsemantics and the stored config representation are unchanged.Tests
Full non-integration suite green locally (732 passed, 1 skipped), including a check that the new tests don't pollute ordering for the previously-affected suites (exceptions, http requests, response/url helpers).
Docs
The configuration page gains a "Stacked Configuration" section (environment-overlay convention, merge rules, explicit overlays), updated in the documentation repository.