Skip to content

SEP-1800: Support a global DATABASE settings block that per-service *__DATABASE__* names override - #1375

Open
peter-o-addo wants to merge 8 commits into
mainfrom
SEP-1800
Open

SEP-1800: Support a global DATABASE settings block that per-service *__DATABASE__* names override#1375
peter-o-addo wants to merge 8 commits into
mainfrom
SEP-1800

Conversation

@peter-o-addo

@peter-o-addo peter-o-addo commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Give per-service settings names deterministic priority over global unprefixed ones, and document one shared DATABASE mount for the PMM side-car.

  • app/core/config.py : split the prefix-strip rewrite into unprefixed and prefixed buckets so {SEP,INVENTORY,TASKS}__* beats the global spelling within each source
  • sidecar/settings.yaml : define DATABASE once with a YAML anchor and alias it from SEP, INVENTORY, and TASKS
  • sidecar/README.md, README.md : document global DATABASE__PASSWORD, per-service override precedence, and the updated side-car mount recipe
  • changelog.d/SEP-1800.config.md : add configuration changelog entry for operators
  • tests/app/core/test_config.py, tests/sidecar/test_embedded_settings.py, tests/sidecar/test_settings_env.py : add precedence and profile coverage; update one settings-env docstring

Tested

  • Set DATABASE__PASSWORD and SEP__DATABASE__PASSWORD in both orderings and confirm SEP always resolves seppw.
  • Mount only DATABASE__PASSWORD and confirm SEP, Inventory, and Tasks all resolve the same password.
  • Mount both DATABASE__PASSWORD and SEP__DATABASE__PASSWORD and confirm SEP uses the per-service value.
  • Set only SEP__DATABASE__PASSWORD and confirm Inventory does not read it.
  • Mount DATABASE__PASSWORD with the side-car profile and confirm the Celery beat store URI uses that password.
  • Confirm sidecar/settings.yaml defines DATABASE once and all three services resolve the same host, name, and port.

Checklist

  • New/modified functions have type hints and rST docstrings
  • New tests added for new features or bug fixes
  • All tests pass locally (make test)
  • Pre-commit hooks pass (make run-pre-commit)
  • Database migrations generated if models changed (make makemigrations)
  • User-facing changes documented (README, inline help, UI text)
  • Configuration changes documented with examples
  • Changelog fragment added under changelog.d/ if the change is user-facing (make changelog-add), or confirmed N/A (internal-only change, or a same-release-cycle fix for an unreleased sibling ticket)

@peter-o-addo peter-o-addo changed the title Sep 1800 SEP-1800: Support a global DATABASE settings block that per-service *__DATABASE__* names override Aug 19, 2026
@peter-o-addo
peter-o-addo marked this pull request as ready for review August 19, 2026 17:44
@peter-o-addo
peter-o-addo requested a review from a team as a code owner August 19, 2026 17:44
Copilot AI balanced review requested due to automatic review settings August 19, 2026 17:44
@peter-o-addo peter-o-addo added the qa in progress Someone is currently testing this PR - do not merge it label Aug 19, 2026
@peter-o-addo

Copy link
Copy Markdown
Contributor Author

1 — Per-service env beats global, regardless of order

Set both DATABASE__PASSWORD=globalpw and SEP__DATABASE__PASSWORD=seppw, then loaded
SEPSettings() with the global variable set first, then with the per-service variable
set first.

global-first: seppw
per-service-first: seppw

2 — One global secret file supplies all three services

Mounted only /tmp/sep1800/secrets/DATABASE__PASSWORD (shared-pw) and loaded
SEPSettings, InventorySettings, and TasksSettings with _secrets_dir pointing at
that directory.

SEPSettings shared-pw
InventorySettings shared-pw
TasksSettings shared-pw

3 — Per-service secret file overrides global

Mounted both DATABASE__PASSWORD (globalpw) and SEP__DATABASE__PASSWORD (seppw),
then loaded SEPSettings from that secrets directory.

SEP: seppw

4 — Foreign prefix is invisible to another service

Set only SEP__DATABASE__PASSWORD=sep-pw in the environment and loaded
InventorySettings() (no global or Inventory-prefixed password).

Inventory: None

5 — Celery beat URI picks up the global mounted password

Mounted DATABASE__PASSWORD (beatpw), ran with the side-car profile, and read
Settings(_secrets_dir=...).CELERY.beat_dburi.

beat: postgresql+psycopg2://sep:beatpw@pmm-server:5432/sep

6 — Side-car profile defines DATABASE once; all services resolve the same connection

Parsed sidecar/settings.yaml to confirm the YAML anchor is shared, then loaded all three
settings classes against the embedded profile.

same object: True
block: {'ENGINE': 'postgresql', 'HOST': 'pmm-server', 'NAME': 'sep', 'PORT': 5432, 'USER': 'sep'}
SEPSettings pmm-server sep 5432
InventorySettings pmm-server sep 5432
TasksSettings pmm-server sep 5432

@peter-o-addo peter-o-addo added qa passed Tests for this PR are completed and successful. and removed qa in progress Someone is currently testing this PR - do not merge it labels Aug 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds deterministic per-service precedence over global database settings and supports one shared side-car database password mount.

Changes:

  • Separates global and prefixed settings before merging.
  • Shares side-car database configuration through YAML aliases.
  • Adds precedence/profile tests and operator documentation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
app/core/config.py Implements prefixed-key precedence.
sidecar/settings.yaml Defines one shared database block.
README.md Documents global settings resolution.
sidecar/README.md Updates side-car mount guidance.
changelog.d/SEP-1800.config.md Adds the operator-facing change note.
tests/app/core/test_config.py Tests source precedence and global passwords.
tests/sidecar/test_embedded_settings.py Tests the shared embedded profile.
tests/sidecar/test_settings_env.py Clarifies shell-export behavior.
Suppressed comments (1)

app/core/config.py:317

  • Update settings_customise_sources’s docstring to describe the new collision rule: prefixed keys override unprefixed keys within each environment, dotenv, or secret-file source, while the existing source ordering still wins across sources. Without that distinction, callers cannot infer the actual precedence contract from this shared API.
                unprefixed: dict[str, Any] = {}
                prefixed: dict[str, Any] = {}
                for key, value in env_source.env_vars.items():
                    stripped = prefix_pattern.sub(r"\1", key)
                    (unprefixed if stripped == key else prefixed)[stripped] = value
                env_source.env_vars = {**unprefixed, **prefixed}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread README.md
Comment on lines +352 to +358
An unprefixed global name such as `DATABASE__PASSWORD` resolves for every prefixed
settings class that reads the same destination — one mounted file reaches SEP,
Inventory, and Tasks when all three share one database. A per-service spelling such
as `SEP__DATABASE__PASSWORD` overrides the global one for that service only; when
both are present, the more specific name wins regardless of ordering. A name spelled
with another class's prefix — `INVENTORY__DATABASE__PASSWORD` read by
`SEPSettings`, say — stays invisible to that class.
@@ -0,0 +1 @@
A global unprefixed settings name such as DATABASE__PASSWORD now resolves for every prefixed service; a per-service {SEP,INVENTORY,TASKS}__* spelling overrides it when both are set. The PMM side-car mount recipe is one DATABASE__PASSWORD file instead of three identical per-service password files.
Comment thread app/core/config.py
Comment on lines +308 to +317
prefix_pattern = re.compile(
f"^{'__'.join(cls.SETTINGS_PREFIXES).lower()}__([a-zA-Z0-9_-]+)$"
)
for env_source in [env_settings, dotenv_settings, secret_settings]:
env_vars = {}
unprefixed: dict[str, Any] = {}
prefixed: dict[str, Any] = {}
for key, value in env_source.env_vars.items():
env_vars[
re.sub(f"^{env_prefix}__([a-zA-Z0-9_-]+)$", r"\1", key)
] = value
env_source.env_vars = env_vars
stripped = prefix_pattern.sub(r"\1", key)
(unprefixed if stripped == key else prefixed)[stripped] = value
env_source.env_vars = {**unprefixed, **prefixed}
@github-actions

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  app/core
  config.py
  app/sep/sync/syncers
  pmm.py
Project Total  

This report was generated by python-coverage-comment-action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python qa passed Tests for this PR are completed and successful.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants