Skip to content

docs(runbooks): note that credential changes require an API restart#365

Merged
SoundMindsAI merged 4 commits into
SoundMindsAI:mainfrom
Jah-yee:relyloop-issue-360
Jun 1, 2026
Merged

docs(runbooks): note that credential changes require an API restart#365
SoundMindsAI merged 4 commits into
SoundMindsAI:mainfrom
Jah-yee:relyloop-issue-360

Conversation

@Jah-yee

@Jah-yee Jah-yee commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Documents that editing cluster_credentials.yaml on a running RelyLoop stack requires a docker compose restart api worker before the API picks up the new credentials. Resolves #360.

Definition of done

  • A clear 'requires API restart' note added to docs/03_runbooks/solr-cluster-registration.md
  • Note explains the cause (cached Settings + @lru_cache) in one sentence
  • No code changes

Background

The Settings accessor for cluster_credentials.yaml uses @lru_cache + @cached_property, memoizing credential resolution at process start. Editing the file on a running stack (or running step 5a of scripts/install.sh after docker compose up -d) means the api serves stale credentials until restarted. The same caching applies to Elasticsearch and OpenSearch credentials, so the note is generalized to all engines.

Resolves SoundMindsAI#360

The Settings accessor used for cluster_credentials.yaml uses
@lru_cache + @cached_property, memoizing credential resolution at
process start. If an operator edits the file on a running stack
(edit to cluster_credentials.yaml or step 5a of scripts/install.sh),
the api keeps using stale credentials until 'docker compose restart
api worker' is run.
@Jah-yee Jah-yee requested a review from SoundMindsAI as a code owner June 1, 2026 11:34

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Solr cluster registration runbook documentation to add a warning that credential edits require restarting the API and worker containers due to caching. The review feedback suggests capitalizing 'api' as 'API' in the text for consistency with the rest of the documentation.

> `cluster_credentials.yaml` is read through a cached `Settings`
> accessor that memoizes its value at process start. If you edit the
> file on a running stack (or run step 5a of `scripts/install.sh` after
> `docker compose up -d`), the api keeps serving the *old* credentials

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For consistency with the rest of the documentation (such as "RelyLoop API container" and "API: POST /api/v1/clusters"), "api" should be capitalized as "API" when referring to the application conceptually.

  > `docker compose up -d`), the API keeps serving the *old* credentials

PR feedback: capitalize 'API' when referring to the application
conceptually, matching 'RelyLoop API container' and 'API: POST'.
@Jah-yee

Jah-yee commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed — thanks for catching that. Changed the api to the API for consistency with the rest of the document.

b19c0f5

@Jah-yee

Jah-yee commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review! Fixed — 'api' → 'API' in the restart command for consistency.

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

Updates the Solr cluster registration runbook to warn operators that changes to ./secrets/cluster_credentials.yaml on a running stack won’t be picked up until the relevant services are restarted (due to process-level settings caching).

Changes:

  • Adds a callout noting that credential edits require restarting services to take effect.
  • Briefly explains that credentials are memoized at process start via cached settings access.
  • Generalizes the warning to cover Elasticsearch and OpenSearch credentials as well.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +27 to +31
> `cluster_credentials.yaml` is read through a cached `Settings`
> accessor that memoizes its value at process start. If you edit the
> file on a running stack (or run step 5a of `scripts/install.sh` after
> `docker compose up -d`), the API keeps serving the *old* credentials
> until you restart: `docker compose restart API worker`.
@SoundMindsAI

Copy link
Copy Markdown
Owner

Thanks for this — the note itself is accurate and genuinely useful, and I verified every claim in it against the codebase: the @cached_property + @lru_cache memoization, the scripts/install.sh step 5a backfill path, and the need to restart both api and worker (both mount cluster_credentials). All correct. 👍

One small thing to revert before merge, and I want to be clear that your original commit had it right:

The restart command needs the service name in lowercase:

docker compose restart api worker

Docker Compose service names are case-sensitive, and the service is defined as api (lowercase) in docker-compose.yml. As currently written (docker compose restart API worker), the command fails with no such service: API, so an operator who copy-pastes it wouldn't actually pick up the new credentials.

For the record, your first commit (30bde6f) had this exactly right — restart api worker. The earlier "capitalize API for consistency" feedback was correct for the prose ("the API keeps serving the old credentials" — that styling matches the rest of the doc and should stay), but it shouldn't extend into the shell command, where the literal service name has to stay lowercase. So this is just walking the command back to your original while keeping the prose capitalization.

Could you flip APIapi on that one command line? After that this is good to merge. Thanks again!

…rt command

Co-Authored-By: Copilot <175728472+Copilot@users.noreply.github.com>
Co-Authored-By: gemini-code-assist[bot] <136328045+gemini-code-assist[bot]@users.noreply.github.com>
@Jah-yee

Jah-yee commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews! ✅ Fixed: changed API workerapi worker to match the actual docker-compose.yml service name. Also noted the caching note. The PR is now correct.

@SoundMindsAI SoundMindsAI merged commit 847948a into SoundMindsAI:main Jun 1, 2026
@SoundMindsAI

Copy link
Copy Markdown
Owner

Thank you! I have approved and merged your PR.

SoundMindsAI added a commit that referenced this pull request Jun 2, 2026
* docs(state): finalize 3 stale MVP2 folders + close #357 + dashboards

Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* docs(dashboards): regenerate after rebase on #385 (reflect both folder-move sets)

Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

---------

Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
@Jah-yee Jah-yee deleted the relyloop-issue-360 branch June 2, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document that cluster credential changes require an API restart

3 participants