Skip to content

SEP-1734: Pass the previous snapshot to RefreshCallback so rebind callbacks can act on what changed - #1298

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

SEP-1734: Pass the previous snapshot to RefreshCallback so rebind callbacks can act on what changed#1298
peter-o-addo wants to merge 8 commits into
mainfrom
SEP-1734

Conversation

@peter-o-addo

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

Copy link
Copy Markdown
Contributor

Summary

Pass both override snapshots into rebind callbacks so endpoint changes can evict clients left on the old origin.

  • lifecycle.py, __init__.py: add SnapshotChange and deliver previous + current to every rebind callback
  • settings_override.py, main.py (SEP): evict previous and current PMM / Inventory / Tasks endpoints on override create, change, and delete
  • main.py (tasks): accept the new change payload in the Nomad rebind callback
  • SEP-1734.fixed.md: note that a replaced endpoint no longer leaves a stale client until shutdown
  • Tests: cover the new payload and create/change/delete eviction paths

Tested

  • Inventory: create, change, then delete the endpoint override and confirm the URL updates each time and returns to the original.
  • Tasks: create, change, then delete the endpoint override and confirm the URL updates each time and returns to the original.
  • PMM: create, change, change only the API key, then delete the override and confirm the endpoint updates, stays the same on key-only change, and returns to the original.

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)

@github-actions github-actions Bot added the python label Aug 6, 2026
@peter-o-addo
peter-o-addo marked this pull request as ready for review August 6, 2026 14:43
Copilot AI review requested due to automatic review settings August 6, 2026 14:43
@peter-o-addo peter-o-addo added the qa in progress Someone is currently testing this PR - do not merge it label Aug 6, 2026

Copilot AI 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.

Pull request overview

This PR updates the settings-override lifecycle so rebind callbacks receive both the previous and current override snapshots (via a new SnapshotChange payload), enabling callbacks to react to what specifically changed (e.g., evicting clients keyed to an old endpoint after an override update/delete).

Changes:

  • Introduces SnapshotChange and updates the core override lifecycle callback signature to deliver previous + current snapshots.
  • Updates SEP and Tasks rebind callbacks to accept the new payload and to evict cached clients for both prior and new endpoints where appropriate.
  • Expands/updates tests to cover the new callback payload and endpoint create/change/delete eviction paths, and adds a changelog fragment.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
app/core/settings_override/lifecycle.py Adds SnapshotChange and updates RefreshCallback/fire_change_callbacks to deliver previous + current snapshots.
app/core/settings_override/init.py Re-exports SnapshotChange from the settings override package.
app/sep/main.py Updates endpoint rebind logic to accept SnapshotChange and evict previous/current endpoints when using registry-cached clients.
app/sep/settings_override.py Updates PMM client invalidation callback to evict on previous/current endpoints using SnapshotChange.
app/tasks/main.py Updates the Nomad rebind callback signature to accept SnapshotChange.
tests/app/sep/test_override_callbacks.py Adds coverage for endpoint and PMM create/change/delete eviction behavior using SnapshotChange.
tests/app/tasks/test_main.py Updates Tasks tests to pass SnapshotChange to the Nomad rebind callback.
tests/app/core/settings_override/test_worker.py Updates worker refresher callback test helper signature to SnapshotChange.
tests/app/core/settings_override/test_lifecycle.py Adds a test asserting fire_change_callbacks delivers a SnapshotChange (including delete semantics).
changelog.d/SEP-1734.fixed.md Documents the runtime-override eviction behavior change for endpoint-based clients.

Comment thread app/core/settings_override/lifecycle.py Outdated
Comment on lines 70 to +74
assert old._session is None # the previous client was drained
finally:
invalidate.assert_not_awaited()
await new.close()
finally:
sep_settings._set_snapshot({})
@peter-o-addo

Copy link
Copy Markdown
Contributor Author

1. Inventory endpoint — HTTP responses

Baseline

GET /api/sep/admin/settings/SEPSettings/INVENTORY_ENDPOINT```

```json
{
  "setting_class": "SEPSettings",
  "key": "INVENTORY_ENDPOINT",
  "value": "http://localhost:8000/api/inventory",
  "has_override": false,
  "reload": "hot"
}

Create — HTTP 200

PATCH /api/sep/admin/settings/SEPSettingsContent-Type: application/json

{"INVENTORY_ENDPOINT":"https://inv-a.example.org"}
[
  {
    "setting_class": "SEPSettings",
    "key": "INVENTORY_ENDPOINT",
    "value": "https://inv-a.example.org/",
    "has_override": true,
    "reload": "hot"
  }
]

Change — HTTP 200

PATCH /api/sep/admin/settings/SEPSettingsContent-Type: application/json

{"INVENTORY_ENDPOINT":"https://inv-b.example.org"}
{
  "setting_class": "SEPSettings",
  "key": "INVENTORY_ENDPOINT",
  "value": "https://inv-b.example.org/",
  "has_override": true
}

Delete — HTTP 204, then GET after delete

DELETE /api/sep/admin/settings/SEPSettings/INVENTORY_ENDPOINT→ HTTP 204
GET /api/sep/admin/settings/SEPSettings/INVENTORY_ENDPOINT```

```json
{
  "setting_class": "SEPSettings",
  "key": "INVENTORY_ENDPOINT",
  "value": "http://localhost:8000/api/inventory",
  "has_override": false
}

Result: PASS — create / change / delete all applied; value restored to base.


2. Tasks endpoint — HTTP responses

Create — HTTP 200

PATCH /api/sep/admin/settings/SEPSettings
{"TASKS_ENDPOINT":"https://tasks-a.example.org"}
{
  "setting_class": "SEPSettings",
  "key": "TASKS_ENDPOINT",
  "value": "https://tasks-a.example.org/",
  "has_override": true
}

Change — HTTP 200

{
  "setting_class": "SEPSettings",
  "key": "TASKS_ENDPOINT",
  "value": "https://tasks-b.example.org/",
  "has_override": true
}

Delete — HTTP 204, then GET

DELETE /api/sep/admin/settings/SEPSettings/TASKS_ENDPOINT → HTTP 204
{
  "setting_class": "SEPSettings",
  "key": "TASKS_ENDPOINT",
  "value": "http://localhost:8000/api/tasks",
  "has_override": false
}

Result: PASS.


3. PMM endpoint — HTTP responses

Baseline

{
  "setting_class": "Settings",
  "key": "PMM",
  "value": {
    "endpoint": "https://127.0.0.1:9090",
    "api_key": "**********",
    "verify_ssl": false
  },
  "has_override": false
}

Create — HTTP 200

PATCH /api/sep/admin/settings/Settings
{"PMM__endpoint":"https://pmm-a.example.org"}
{
  "setting_class": "Settings",
  "key": "PMM",
  "value": {
    "endpoint": "https://pmm-a.example.org",
    "api_key": "**********",
    "verify_ssl": false
  },
  "has_override": true
}

Change — HTTP 200

{
  "value": {
    "endpoint": "https://pmm-b.example.org",
    "api_key": "**********",
    "verify_ssl": false
  },
  "has_override": true
}

Same-endpoint credential change — HTTP 200

PATCH /api/sep/admin/settings/Settings
{"PMM__api_key":"test-key-for-rebind"}
{
  "value": {
    "endpoint": "https://pmm-b.example.org",
    "api_key": "**********",
    "verify_ssl": false
  },
  "has_override": true
}

Endpoint unchanged; only credential override applied.

Delete — HTTP 204 × 2, then GET

DELETE /api/sep/admin/settings/Settings/PMM__api_key → HTTP 204
DELETE /api/sep/admin/settings/Settings/PMM__endpoint → HTTP 204
{
  "value": {
    "endpoint": "https://127.0.0.1:9090",
    "api_key": "**********",
    "verify_ssl": false
  },
  "has_override": false
}

Result: PASS.

@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 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  app/core
  config.py
  app/core/settings_override
  lifecycle.py
  app/sep
  main.py
  settings_override.py
  app/sep/apps/mysql_backups/restore
  deps.py
  app/sep/sync/syncers
  pmm.py
  app/tasks
  main.py
Project Total  

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

@yyyyyyyan

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Copilot AI and others added 2 commits August 7, 2026 15:07
# Conflicts:
#	app/core/settings_override/lifecycle.py

Co-authored-by: yyyyyyyan <24644216+yyyyyyyan@users.noreply.github.com>
Co-authored-by: yyyyyyyan <24644216+yyyyyyyan@users.noreply.github.com>

Copilot AI commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflict in app/core/settings_override/lifecycle.py — kept both the new _drain_cancelled_seed_task helper from main and our SnapshotChange class. Also addressed the two review comments (docstring imperative mood and test cleanup reliability). Pushed in commit 331ddb9.

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.

4 participants