Migrate configuration from environment variables to a YAML deployment file - #821
Open
MattyTheHacker wants to merge 36 commits into
Open
Migrate configuration from environment variables to a YAML deployment file#821MattyTheHacker wants to merge 36 commits into
MattyTheHacker wants to merge 36 commits into
Conversation
Stands up `config/_schema.py` as a faithful translation of the existing StrictYAML schema, declaring every configuration setting as a pydantic model. Nothing is wired in yet: both schemas currently coexist, so the two can be diffed against each other before the loading path is switched over. The pydantic schema is verified to match the StrictYAML schema exactly: 32 leaf settings, identical keys and identical required-ness. Deliberate deviations from the StrictYAML schema: - The `logging:discord-channel` section is now genuinely optional. Previously it carried a default that omitted its own required `webhook-url` key, so writing a `logging:` section without a `discord-channel:` section failed to load, reporting a location of `"None", line 1`. - Empty delay/interval strings are rejected rather than parsed as a zero-length duration, which would cause any task looping upon it to spin without pausing. - Values failing to match the delay/interval format now raise directly, rather than falling through to pydantic, which would otherwise also accept ISO-8601 durations and `HH:MM:SS` strings that the documented format does not allow. Help text, whether a restart is required, and whether a value is secret are now declared as field metadata, so that the `/config` command can be driven from the schema itself rather than from a separately maintained mapping. Secrets are held as `SecretStr`, keeping them out of reprs, logs and dumps. Also enables the pydantic mypy plugin. Model declarations are exempted from `disallow_any_explicit`, which is incompatible with subclassing `BaseModel`.
Reverses the order that time resolutions must be given in, within delay/interval strings. Durations are now written largest-unit-first, so `1h30m` is accepted and `30m1h` is not; previously this was the other way around, inherited from the StrictYAML implementation. The full format is `<days>d<hours>h<minutes>m<seconds>s`. Replaces the module-wide `explicit-any` exemption with a per-class `# type: ignore[explicit-any]` on each model. Subclassing Pydantic's `BaseModel` is inherently incompatible with `disallow_any_explicit`, because `BaseModel`'s own API surface exposes explicit `Any`, but suppressing the error code for the whole module would also hide any genuine use of `Any` added here later. Ignoring it per-class keeps the check active everywhere else within the module.
Adds `config/_document.py`, which owns every interaction with `tex-bot-deployment.yaml` as a document: locating it, parsing it via ruamel.yaml so that comments & formatting survive, writing it back, and mapping validation failures onto the lines that caused them. It is not wired in yet. Parsing a file and writing it straight back is byte-identical, and changing individual values leaves surrounding comments (including trailing inline comments) intact, which is what allows `/config set` to edit a hand-written file without destroying it. Writes go to a temporary file alongside the destination and are then moved into place, so that failing partway through cannot leave a truncated configuration file behind, and so that a reader never observes a half-written file. Validation failures are rendered with the file & line that caused them, resolved from ruamel's position data. Where a key is absent entirely, the line of the deepest resolvable parent is reported instead, so the message still points at the relevant section. Offending values are excluded from these messages, so that secrets cannot reach logs or a Discord channel. Locating the file uses a single `TEX_BOT_CONFIG_PATH` environment variable, falling back to `tex-bot-deployment.yaml` in the project root, rather than the twelve interchangeable environment variables previously proposed.
Adds `config/_accessor.py`, joining the schema & the document together: it reads the configuration file, validates it, and holds the result as a single immutable snapshot. It is not wired in yet. Reloading replaces that snapshot by rebinding one reference, so every reader sees either the whole of the previous configuration or the whole of the new one. This replaces the previously proposed approach of mutating settings one at a time through around twenty-five separate reload methods, where a failure partway through would leave the configuration half-applied with no way back. A reload that fails, because the file is unreadable or contains invalid settings, leaves the running configuration untouched, so that a bad edit cannot take a running bot down. Reloading also reports which settings actually changed, which is what the file watcher will use to decide what needs re-applying. Settings are reached by section as typed attributes, so `settings.discord.bot_token` is known to be a `SecretStr` and `settings.commands.strike.timeout_duration` a `timedelta`, rather than every value being typed as `object`. The token guard that prevented reading the bot token once running is not carried over. Holding the token as a `SecretStr` addresses the same concern more directly: it cannot be printed, logged or interpolated by accident, and reading it now requires an explicit `get_secret_value()` call. Also reports an explicitly given but non-existent configuration file path as a `SettingsFileNotFoundError`, rather than letting a bare `FileNotFoundError` escape.
Switches TeX-Bot over to the new configuration package and removes the old environment-variable loader, so the branch boots again. Configuration is now read from a `tex-bot-deployment.yaml` file, validated against the pydantic schema, and reached through typed nested attributes: what was `settings["STATISTICS_DAYS"]` returning `object` is now `settings.commands.stats.lookback_period` returning a `timedelta`. All 120 call sites across 15 files are migrated, and the whole project now type-checks cleanly for the first time on this branch. Deletes `config.py`, `config/_settings/`, `config/constants.py` and the hand-written `stubs/strictyaml/` type stubs, and drops the `strictyaml`, `aiopath` & `anyio` dependencies. The latter two were never imported by anything. Several values change type at the point of use, so those call sites are adapted rather than merely renamed: - Links are `HttpUrl`, so are converted where a `str` is required. - The bot token & the members-list authentication cookie are `SecretStr`, and are now unwrapped explicitly at the two places that genuinely need their value. - Reminder intervals are `timedelta` rather than a mapping of keyword arguments, so `tasks.loop()` is given `seconds=...` directly. - `membership-dependent-roles` defaults to being empty rather than absent, matching the behaviour of the loader being replaced, so that consumers need no null check before iterating it. Response messages stay in their own JSON file, loaded by `config/_messages.py`. They are a body of content rather than settings, so they are deliberately neither validated by the settings schema nor editable through the `/config` command. Logging is applied from the loaded settings by `config/_logging.py`, replacing handlers rather than adding to them, so that reloading cannot accumulate duplicates. Restores `ImproperlyConfiguredError`, which two existing database migrations import and therefore cannot be removed. Adds `tex-bot-deployment.example.yaml` documenting the new format, and ignores the real configuration file, which holds the bot token, along with the temporary file written beside it whilst it is being rewritten.
The image build has been failing since `config.py` was replaced: the Dockerfile still copied that file, and never copied the `config/` package that replaced it. Also excludes the deployment configuration file from the build context, so that a local configuration holding a real bot token cannot be captured in an image layer, along with the temporary file written beside it whilst it is being rewritten.
Replacing the configuration file by renaming a temporary file over it is atomic, but it is rejected when the file has been mounted into a container individually, because a rename cannot replace a mount point. That is the obvious way to supply the file to the published container image, so writing a setting would have failed for most deployments. Where the rename is rejected, the file is now written directly instead. Doing so is not atomic, but a torn write is only possible if the process dies during it, whereas being unable to save at all would have been certain. Also gives the image a dedicated `/app/data` directory for the configuration file, kept apart from the application code so it can be mounted as a directory rather than as an individual file. Mounting the directory avoids the rejected rename entirely, keeps the atomic path as the one normally taken, and means a named volume inherits ownership from the image and is writable without any further setup. `TEX_BOT_CONFIG_PATH` points there by default within the image; running outside a container is unaffected.
Adds the `/config` command group, with a `reload` subcommand that reads the deployment configuration file again and applies every change that can be applied while TeX-Bot is running. Both are restricted to committee members. A reload that fails, because the file cannot be read or contains invalid settings, changes nothing at all and reports why, quoting the file and line responsible. Most settings take effect immediately, because they are read from the settings accessor at the point they are used. The exception is a background task's interval, which is captured when its cog class is defined, so cogs are now offered each reload through an `on_config_reloaded` hook and re-apply anything they hold a copy of. The three task cogs use it to change their interval, and to start or stop themselves, without a restart. Only three settings now require a restart, down from eight: - `discord:bot-token` and `discord:main-guild-id`, which are used to establish the connection and to populate the shortcut accessors during startup. - `reminders:send-introduction-reminders:enabled`, because setting it to `interval` also clears the record of which members have already been sent a one-off reminder. That is a database side effect which should not happen implicitly during a reload, so it is deliberately left to a restart. Where a changed setting needs a restart, the reload still succeeds and applies everything else, and the response says which settings are waiting. Whether a setting requires a restart is now read from the schema, rather than being tracked separately, so the two cannot disagree.
Reverts applying background task settings while TeX-Bot is running. Whether a task runs is decided when its cog is initialised, and how often it runs is fixed when its cog class is defined, so keeping either in step with the configuration meant starting, stopping & restarting tasks underneath themselves. Reporting that a restart is needed is easier to reason about, and to predict, than a task being torn down and recreated part-way through its work. `/config reload` therefore now reports the `enabled` & `interval` of all three recurring tasks as needing a restart, alongside the bot token and main guild ID. Their `delay` settings are unaffected: those are read from the settings accessor inside the task body, so they continue to take effect immediately, and flagging them would send committee members off to restart TeX-Bot for no reason. The reason for each of these is documented in the example configuration file, where it will be read while the file is being filled in. Removes the machinery that existed only to re-apply settings to running tasks: the per-task helper, the `on_config_reloaded` hook offered to every cog, and the loop that dispatched to it. With nothing left to dispatch to, working out which changed settings need a restart now lives beside the reload itself, and the command calls straight into it.
Adds 156 tests covering the settings schema, the configuration file reader & writer, the settings accessor, the messages accessor, applying the logging configuration, and reloading. Coverage of the config package is 98%; the remainder is the database setup performed at start-up. Writing these found three defects: - Reloading raised an unhandled exception when the configuration file was missing or malformed. `/config reload` caught `OSError`, but neither `SettingsFileNotFoundError` nor `InvalidSettingsFileError` inherits from it, so the command failed rather than reporting the problem. Both are now caught explicitly. - A duration given as a bare number was accepted as a count of seconds, so `timeout-duration: 24` silently meant 24 seconds rather than the 24 hours its author would have intended. This was inconsistent with the string `'24'`, which was already rejected for having no unit; a unit is now always required. - Pytest was reading none of its configuration, because it only looks in `[tool.pytest.ini_options]` and the settings were in a plain `[tool.pytest]` table. The settings accessor is a module-level singleton, and reloading reports what changed relative to what was loaded before, so the tests that reload through the package's public surface replace it with an empty accessor beforehand. Without that the results would depend upon the order the tests happened to run in; the suite passes in shuffled order.
Reviewing every NOTE left within the configuration package found several
behaviours that nothing exercised, & five defects amongst them:
* A zero-length interval (`0s`) was accepted, despite the empty string
being rejected to prevent exactly that. Each interval becomes the loop
period of a recurring task, so `interval: 0s` produced a task that ran
continuously without ever pausing. Intervals are now declared as
`PositiveTimeDelta`; a delay of zero remains meaningful & is unaffected.
* `send-introduction-reminders: 0` was accepted as `false`, while the `1`
its author would pair it with was rejected.
* A JSON object was accepted as a set of response messages, & silently
reduced to its own keys, discarding every message it held.
* The example deployment configuration did not validate: its placeholder
values for optional settings were rejected, so anybody copying it &
filling in the two required values could not start TeX-Bot at all.
* `SlashCommandGroup.command()` was stubbed with an unsolvable type
variable, resolving to `Never`, which removed every command within a
group from type-checking entirely.
Also adds the regression tests that the previously-fixed `/config reload`
error handling never had, along with tests for the round-trip line width,
the `\Z` pattern anchors, the missing-key sentinel used to detect changes,
logger propagation, & the constraints upon each individual setting.
Each new test guarding a NOTE was verified by reverting the behaviour it
describes & confirming that it, & only it, failed.
Committee members can now read & change individual settings from within
Discord, without editing the deployment configuration file by hand.
Every change is applied to a copy of the file & validated before anything
is written, so a value that would be rejected leaves both the file & the
running configuration exactly as they were. The file is read again before
each change rather than the copy held in memory being rewritten, so an edit
made by hand in the meantime is kept rather than silently reverted. The
comments & formatting of the file survive being rewritten, as before.
Values are read exactly as the same text would be if it had been written
into the file directly, with two exceptions worth naming:
* A setting declared to hold text stays text, so an organisation ID of
`1234` is not written as a number & then rejected for being one. This
is derived from the schema, so it cannot drift from what is declared.
* Text holding a colon, or beginning with a `#`, is kept as the text it
plainly is rather than being read as a mapping or as a comment.
Removing a setting needs no knowledge of which settings are required: the
configuration that removing it would produce simply does not validate.
Also fixes a crash this uncovered: reporting a validation failure against a
setting that had just been added to the document raised `TypeError`, & then
`KeyError`, from `line_number_of()`, because such a setting has no line
within the file it was parsed from. Every rejected new setting would have
surfaced as an unhandled exception rather than as an explanation.
Autocomplete matches anywhere within a setting's name, rather than only at
its beginning, because each name is prefixed with the section holding it.
Changing a setting from within Discord reads the configuration file afresh
& applies the change on top of it, so the two kinds of change compose:
* Where they affect different settings, both are kept.
* Where they affect the same setting, the change made from within Discord
wins, being the later of the two & the one somebody is waiting upon.
* A setting added or removed by hand is likewise kept.
* A mistake made by hand anywhere within the file blocks the change &
is reported, rather than being written back.
Each of these is now covered, & each was checked by making the change apply
to the configuration already loaded instead: four of the six then fail.
Also drops the copy taken before applying a change. Reading the file again
already produces a document that nothing else holds a reference to, so
copying it was pure duplication; the safety it was there for comes from
reading afresh, not from the copy. This removes roughly an eighth of the
time a single change takes (16ms, against a Discord round-trip).
Changing a setting from within Discord previously read the file afresh & merged the change into whatever it found there. That quietly applied somebody else's unrelated edits as a side effect of an unrelated change: the response named only the setting its author had asked for, yet the reload behind it applied the whole file. An edit to a setting fixed at start-up was worse still, reporting that TeX-Bot had to be restarted for a setting whoever ran the command had never touched. Both `/config set` & `/config unset` now refuse a file that has been edited since it was last loaded, & say to run `/config reload` first. Reloading replaces the document held alongside the settings, so it clears the refusal even for an edit that changed no setting at all. `/config get` keeps working, but says when the value it is showing may no longer match the file, rather than showing a stale value without comment. It reports what TeX-Bot is running, which is the honest answer. The comparison is made between the documents the files parse to, rather than their raw text, so a file rewritten with different line endings is not mistaken for one whose settings were edited. All of this leaves every `/config` command acting upon the configuration that TeX-Bot has actually loaded, which is a far simpler rule to hold in mind than the merge it replaces.
"/config get" already said when the configuration file had been edited since it was last loaded, but not what the edit had changed the setting to, leaving whoever asked to go & read the file to find out. It now shows both the value TeX-Bot is running & the value the file holds, so the difference that reloading would apply can be seen at a glance. An edit that leaves the file invalid is explained rather than shown, since there is no value to show from a file that would be refused, & the reason it would be refused is what needs fixing before reloading. An edit to a secret is reported as differing without either value being rendered. Where the file has been edited but not in a way that changes the setting being viewed, that is said plainly, rather than implying the value shown might be stale when it is not. Also fixes an unhandled exception this uncovered: viewing a setting within an optional section that had been left out of the file raised `KeyError`, because such a section collapses to a single empty value once the settings are flattened & so has no entry of its own. Both settings within the Discord log-channel section are offered by autocomplete, so any deployment without that section would have hit this by choosing one.
Every piece of documentation still described the environment variables that
the deployment configuration file replaced, so anybody following it would
have configured a bot that read none of it.
README.md now explains the configuration file, what must be filled in
before TeX-Bot will start, how to mount it into the container, & how to
view & change settings with the `/config` commands. Each error code &
repeated task now names the setting responsible rather than the variable
that used to hold it, & a table maps every old variable onto the setting
that replaced it.
CONTRIBUTING.md describes the `config` package a module at a time, in place
of the `config.py` it points at, which no longer exists. Its list of cogs
was missing eight of them, including the one added for `/config`.
Also corrects three things that were wrong rather than merely outdated:
* The members-list cookie is named `.AspNet.SharedCookie`, which is what
the code sends & reads. Both the schema & the example configuration
called it `.ASPXAUTH`, which appears nowhere else in the project, & the
schema's wording is shown by `/config get`.
* The get-roles reminder interval was documented as defaulting to every
24 hours, where it has always defaulted to every 6 hours.
* The example configuration was the only file in the repository failing
the yamllint hook, so this branch would have failed CI. Its settings
are deliberately ordered to be read from the top down, which the
alphabetical ordering that yamllint requires would destroy, so the
deployment configuration is now excluded from that check.
`.env.example` is deleted, along with the linter that had nothing left to
check & the re-inclusion that would have copied a `.env` file into the
container image. `python-dotenv` stays a dependency: two database
migrations import it, & a migration that has already been applied cannot
be edited.
MattyTheHacker
added
enhancement
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
MattyTheHacker
had a problem deploying
to
publish
August 7, 2026 15:30 — with 
GitHub Actions
Failure
GitHub Actions
Failure
This was
linked to
issues
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #221 due to extensive conflicts
Moves configuration to a yaml and allows for editing by discord commands. Strictyaml has been yeeted since it doesn't seme like it's being maintained anymore.
Changes can still be made manually by editing the file but this will require a reload command be run in discord and is not recommended
As minimal as possible changes require a restart, only ones that do are server ID, bot token and task scheduling things that in theory could be done without a reboot but caused many many problems in trying to do so