Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 8c69911252 | |||
| 7b3e2ebace |
@@ -0,0 +1,177 @@
|
|||||||
|
# Design: Unify Saved Tasks on SSH Services
|
||||||
|
|
||||||
|
**Change:** `unify-tasks-on-services`
|
||||||
|
**Phase:** design
|
||||||
|
**Date:** 2026-06-19
|
||||||
|
|
||||||
|
## 1. Architecture overview
|
||||||
|
|
||||||
|
```
|
||||||
|
┌─────────────────────────────────────┐
|
||||||
|
│ saved_tasks (global, reusable) │
|
||||||
|
│ default_service_id → ssh_tasks │
|
||||||
|
└─────────────────────────────────────┘
|
||||||
|
│ │
|
||||||
|
Actions page │ │ SSH task widget
|
||||||
|
▼ ▼
|
||||||
|
┌─────────────────────────────────────┐
|
||||||
|
│ run_saved_task(store, task, svc) │ ← shared helper
|
||||||
|
│ build client → run → log │
|
||||||
|
└─────────────────────────────────────┘
|
||||||
|
│
|
||||||
|
▼
|
||||||
|
┌─────────────────────────────────────┐
|
||||||
|
│ service_task_runs (one history) │
|
||||||
|
└─────────────────────────────────────┘
|
||||||
|
```
|
||||||
|
|
||||||
|
Both the Actions runner and the SSH task widget call one shared helper, so there
|
||||||
|
is a single execution path and a single history table.
|
||||||
|
|
||||||
|
## 2. Shared execution helper
|
||||||
|
|
||||||
|
New: `backend/src/media_library_viewer_api/services/task_runner.py`
|
||||||
|
|
||||||
|
```python
|
||||||
|
from dataclasses import dataclass
|
||||||
|
from media_library_viewer_api.services.settings_store import SettingsStore
|
||||||
|
from media_library_viewer_api.widgets.sources import ServiceRecord, _build_ssh_client
|
||||||
|
|
||||||
|
@dataclass
|
||||||
|
class TaskRunResult:
|
||||||
|
exit_status: int
|
||||||
|
stdout: str
|
||||||
|
stderr: str
|
||||||
|
duration_ms: int
|
||||||
|
status: str # "success" | "failure" | "timeout" | "error"
|
||||||
|
error: str
|
||||||
|
|
||||||
|
def run_saved_task(
|
||||||
|
store: SettingsStore,
|
||||||
|
task: dict,
|
||||||
|
service: ServiceRecord,
|
||||||
|
*,
|
||||||
|
request_id: str = "",
|
||||||
|
) -> TaskRunResult:
|
||||||
|
"""Run a saved task on an ssh_tasks service instance and log it.
|
||||||
|
|
||||||
|
Builds the SSH client from the service record, renders the command (shell or
|
||||||
|
python3 -c), runs it with the service's timeout, appends a service_task_runs
|
||||||
|
row, and returns the result.
|
||||||
|
"""
|
||||||
|
...
|
||||||
|
```
|
||||||
|
|
||||||
|
- The widget adapter (`SshTaskWidgetSource.fetch`) is refactored to call
|
||||||
|
`run_saved_task`, removing its inline copy.
|
||||||
|
- `routers/tasks.py` `run_task` calls `run_saved_task` instead of
|
||||||
|
`_client_for_machine` + `record_task_run`.
|
||||||
|
- `_build_ssh_client` (currently private in `widgets/sources.py`) is promoted to
|
||||||
|
the helper module or a shared location so both callers use it.
|
||||||
|
|
||||||
|
## 3. Data model changes
|
||||||
|
|
||||||
|
### 3.1 `saved_tasks`
|
||||||
|
|
||||||
|
```sql
|
||||||
|
-- default_machine_id replaced by default_service_id
|
||||||
|
ALTER TABLE saved_tasks RENAME COLUMN default_machine_id TO default_service_id;
|
||||||
|
```
|
||||||
|
|
||||||
|
In SQLite (3.25+) `RENAME COLUMN` is supported. The column still stores an id,
|
||||||
|
now pointing at `services.id` (an `ssh_tasks` instance) instead of a machine.
|
||||||
|
|
||||||
|
### 3.2 `saved_task_runs` dropped
|
||||||
|
|
||||||
|
```sql
|
||||||
|
DROP TABLE IF EXISTS saved_task_runs;
|
||||||
|
```
|
||||||
|
|
||||||
|
All history lives in `service_task_runs` (added in the service-registry change).
|
||||||
|
The `record_task_run` / `list_task_runs` methods on `SettingsStore` are removed.
|
||||||
|
|
||||||
|
## 4. Backend API
|
||||||
|
|
||||||
|
### `routers/tasks.py`
|
||||||
|
|
||||||
|
| Method | Path | Change |
|
||||||
|
|--------|------|--------|
|
||||||
|
| GET | `/api/tasks` | Unchanged (task now carries `default_service_id`). |
|
||||||
|
| POST | `/api/tasks` | `TaskInput.default_service_id` replaces `default_machine_id`. |
|
||||||
|
| PUT | `/api/tasks/{id}` | Same field rename. |
|
||||||
|
| DELETE | `/api/tasks/{id}` | Unchanged. |
|
||||||
|
| GET | `/api/tasks/{id}/runs` | Reads `service_task_runs` (filtered by `task_id`). |
|
||||||
|
| POST | `/api/tasks/run?service_id=...` | `service_id` replaces `machine_id`; resolves an `ssh_tasks` service (override) or the task's `default_service_id`; calls `run_saved_task`. |
|
||||||
|
|
||||||
|
`_resolve_machine_for_task` and `_client_for_machine` are removed (replaced by
|
||||||
|
service resolution + the shared helper).
|
||||||
|
|
||||||
|
### Resolution + validation
|
||||||
|
|
||||||
|
- `run_task`: load the task; if `service_id` query param is given, use it
|
||||||
|
(override), else use `task.default_service_id`; load the `ssh_tasks` service
|
||||||
|
record; build a `ServiceRecord` (decrypt secrets); call `run_saved_task`.
|
||||||
|
- 400 if the task is disabled; 400 if no service resolves; 404 if the task or
|
||||||
|
service is missing.
|
||||||
|
|
||||||
|
## 5. Frontend
|
||||||
|
|
||||||
|
### 5.1 Types
|
||||||
|
|
||||||
|
`SavedTask` / `SavedTaskInput` / `SavedTaskRun` (`frontend/src/types/index.ts`):
|
||||||
|
|
||||||
|
- `default_machine_id` → `default_service_id`.
|
||||||
|
- `SavedTaskRun` fields align with `service_task_runs` (`service_id`,
|
||||||
|
`exit_status`, `stdout_tail`, …).
|
||||||
|
|
||||||
|
### 5.2 API client + hooks
|
||||||
|
|
||||||
|
- `runTask(taskId, serviceId?)` sends `service_id`.
|
||||||
|
- `fetchSavedTaskRuns(taskId)` reads `/api/tasks/{id}/runs` (now
|
||||||
|
`service_task_runs`-backed).
|
||||||
|
|
||||||
|
### 5.3 Actions page
|
||||||
|
|
||||||
|
- Task editor: "Default service" `<Select>` lists `ssh_tasks` service instances
|
||||||
|
(via `useServiceInstances("ssh_tasks")`), not machines.
|
||||||
|
- Run dialog: "Run on" `<Select>` lists `ssh_tasks` instances (override).
|
||||||
|
- Run history: reads the task's `service_task_runs`.
|
||||||
|
- `useMonitoringSettings` removed from the Actions page (no longer needed).
|
||||||
|
|
||||||
|
## 6. Migration and breaking changes
|
||||||
|
|
||||||
|
- **DB:** `saved_tasks.default_machine_id` renamed to `default_service_id`
|
||||||
|
(existing values become stale references to machine ids; inert — the user
|
||||||
|
re-points). `saved_task_runs` dropped.
|
||||||
|
- **Local execution removed.** Deployments relying on local tasks must use an
|
||||||
|
`ssh_tasks` service (e.g. pointing at localhost with a key).
|
||||||
|
- **Changelog + README** note the breaking change.
|
||||||
|
|
||||||
|
## 7. File-level plan
|
||||||
|
|
||||||
|
### Create (backend)
|
||||||
|
|
||||||
|
- `services/task_runner.py` — `run_saved_task` shared helper.
|
||||||
|
|
||||||
|
### Modify (backend)
|
||||||
|
|
||||||
|
- `services/settings_store.py` — rename column; drop `saved_task_runs` +
|
||||||
|
`record_task_run` / `list_task_runs` (task-run flavor).
|
||||||
|
- `routers/tasks.py` — service resolution; call `run_saved_task`; `service_id`
|
||||||
|
param; read `service_task_runs`.
|
||||||
|
- `widgets/sources.py` — `SshTaskWidgetSource.fetch` delegates to
|
||||||
|
`run_saved_task`.
|
||||||
|
|
||||||
|
### Modify (frontend)
|
||||||
|
|
||||||
|
- `types/index.ts` — field rename + `SavedTaskRun` alignment.
|
||||||
|
- `api/client.ts` — `runTask` sends `service_id`.
|
||||||
|
- `pages/Actions.tsx` — service selectors + history source.
|
||||||
|
|
||||||
|
## 8. Slice boundaries
|
||||||
|
|
||||||
|
1. **Backend** — `run_saved_task` helper; saved_tasks column rename; tasks router
|
||||||
|
rewired; widget delegates; `saved_task_runs` dropped; tests.
|
||||||
|
2. **Frontend** — types + API + Actions page rewire; tests.
|
||||||
|
|
||||||
|
Estimated ~600–800 changed lines across two PRs.
|
||||||
@@ -0,0 +1,78 @@
|
|||||||
|
# Proposal: Unify Saved Tasks on SSH Services
|
||||||
|
|
||||||
|
**Change:** `unify-tasks-on-services`
|
||||||
|
**Phase:** proposal
|
||||||
|
**Date:** 2026-06-19
|
||||||
|
**Status:** awaiting review (design only — no implementation yet)
|
||||||
|
|
||||||
|
## Context and problem
|
||||||
|
|
||||||
|
Saved tasks (the Actions page) currently have **two execution paths**:
|
||||||
|
|
||||||
|
1. **Actions page** → resolves a *machine* (`default_machine_id`) → runs via
|
||||||
|
`_client_for_machine` → logs to `saved_task_runs`.
|
||||||
|
2. **SSH task widget** → resolves an `ssh_tasks` *service instance* → runs via
|
||||||
|
`_build_ssh_client` → logs to `service_task_runs`.
|
||||||
|
|
||||||
|
Same saved-task records, two runners, two history tables, two target models. This
|
||||||
|
is the leftover inconsistency from the service-registry change (design §12): the
|
||||||
|
widget was migrated to services but the Actions page was not.
|
||||||
|
|
||||||
|
## Proposal
|
||||||
|
|
||||||
|
Migrate the Actions page onto the same `ssh_tasks` service model the widget
|
||||||
|
already uses, so there is **one execution path** and **one history table**.
|
||||||
|
|
||||||
|
- Saved tasks gain `default_service_id` (replaces `default_machine_id`), pointing
|
||||||
|
at an `ssh_tasks` service instance.
|
||||||
|
- The Actions runner resolves an `ssh_tasks` service (the task's default, or an
|
||||||
|
explicit run-time override), builds the SSH client from the service record, runs
|
||||||
|
the task, and logs to `service_task_runs`.
|
||||||
|
- `saved_task_runs` is dropped; both the Actions page and the widget read
|
||||||
|
`service_task_runs`.
|
||||||
|
- Local (API-host) task execution is dropped — all tasks run over SSH against
|
||||||
|
`ssh_tasks` services.
|
||||||
|
|
||||||
|
## Goals
|
||||||
|
|
||||||
|
- One execution path for saved tasks (Actions page + widget share it).
|
||||||
|
- One run-history table (`service_task_runs`).
|
||||||
|
- Tasks target `ssh_tasks` service instances, consistent with the rest of the
|
||||||
|
service registry.
|
||||||
|
- Run-time override preserved: a task can be run against any `ssh_tasks` instance.
|
||||||
|
|
||||||
|
## Non-goals
|
||||||
|
|
||||||
|
- **No change to the jobs router** (`/api/jobs/run`, the `disk_usage` template,
|
||||||
|
etc.). That stays machine-based for the File Browser's on-demand SSH checks.
|
||||||
|
- **No machine/service unification** (follow-up #3). Machines still own File
|
||||||
|
Browser + node_exporter transport.
|
||||||
|
- **No local execution mode.** Dropped per decision; tasks are SSH-only.
|
||||||
|
- **No automatic data migration** of `default_machine_id` → `default_service_id`.
|
||||||
|
Break backwards compatibility (consistent with the service-registry change):
|
||||||
|
existing tasks lose their default target and the user re-points them.
|
||||||
|
|
||||||
|
## Decisions (from grilling)
|
||||||
|
|
||||||
|
| Topic | Decision |
|
||||||
|
|-------|----------|
|
||||||
|
| Local execution | **SSH-only.** Drop local mode; `ssh_tasks` services handle all task execution. |
|
||||||
|
| Run history | **`service_task_runs` only.** Drop `saved_task_runs`. |
|
||||||
|
| Run-time override | **Keep.** A task can run against any `ssh_tasks` instance at run time. |
|
||||||
|
|
||||||
|
## Risks
|
||||||
|
|
||||||
|
- **Breaking upgrade.** Existing tasks lose `default_machine_id`; users re-point
|
||||||
|
to an `ssh_tasks` service. Document in changelog.
|
||||||
|
- **Local-mode loss.** Any deployment relying on local task execution must set up
|
||||||
|
an SSH loopback (or an ssh_tasks service pointing at localhost with a key) to
|
||||||
|
keep running local tasks.
|
||||||
|
- **Shared execution code.** The Actions runner and the widget must share one
|
||||||
|
`run_saved_task` helper to avoid divergence; extracting it is the core refactor.
|
||||||
|
|
||||||
|
## Out of scope
|
||||||
|
|
||||||
|
- Machine/service unification (follow-up #3).
|
||||||
|
- Migrating the jobs router (`/api/jobs`) off machines.
|
||||||
|
- A UI for browsing `service_task_runs` across all services (the service page
|
||||||
|
already shows per-instance history; the Actions page shows per-task history).
|
||||||
@@ -0,0 +1,105 @@
|
|||||||
|
# Tasks: Unify Saved Tasks on SSH Services
|
||||||
|
|
||||||
|
**Change:** `unify-tasks-on-services`
|
||||||
|
**Phase:** tasks
|
||||||
|
**Date:** 2026-06-19
|
||||||
|
|
||||||
|
## Review workload forecast
|
||||||
|
|
||||||
|
| Field | Value |
|
||||||
|
|-------|-------|
|
||||||
|
| Estimated changed lines | ~600–800 |
|
||||||
|
| Chained PRs recommended | Yes (2 PRs) |
|
||||||
|
| Chain strategy | stacked-to-main |
|
||||||
|
|
||||||
|
## Slice 1: Backend — shared runner + service-based tasks
|
||||||
|
|
||||||
|
**Goal:** One execution path; tasks target ssh_tasks services; one history table.
|
||||||
|
|
||||||
|
- [ ] **1.1 Add shared `run_saved_task` helper**
|
||||||
|
- Files: `backend/src/media_library_viewer_api/services/task_runner.py` (new)
|
||||||
|
- Lines: ~90
|
||||||
|
- Details: `run_saved_task(store, task, service, *, request_id)` builds the SSH
|
||||||
|
client from the service record (promote `_build_ssh_client`), renders the
|
||||||
|
command, runs with the service timeout, appends a `service_task_runs` row,
|
||||||
|
returns a `TaskRunResult`.
|
||||||
|
- [ ] **1.2 Rename saved_tasks column**
|
||||||
|
- Files: `services/settings_store.py` (modify)
|
||||||
|
- Lines: ~20
|
||||||
|
- Details: `default_machine_id` → `default_service_id` (ALTER TABLE RENAME
|
||||||
|
COLUMN on startup; update `_row_to_task`, `_normalize_task_payload`,
|
||||||
|
`upsert_task`).
|
||||||
|
- [ ] **1.3 Drop saved_task_runs**
|
||||||
|
- Files: `services/settings_store.py` (modify)
|
||||||
|
- Lines: ~-60
|
||||||
|
- Details: `DROP TABLE IF EXISTS saved_task_runs`; remove `record_task_run`
|
||||||
|
and `list_task_runs` (task flavor).
|
||||||
|
- [ ] **1.4 Rewire tasks router**
|
||||||
|
- Files: `routers/tasks.py` (modify)
|
||||||
|
- Lines: ~70
|
||||||
|
- Details: `TaskInput.default_service_id`; `run_task` takes `service_id`
|
||||||
|
(override), resolves an ssh_tasks service, calls `run_saved_task`;
|
||||||
|
`/api/tasks/{id}/runs` reads `service_task_runs`. Remove
|
||||||
|
`_resolve_machine_for_task` and `_client_for_machine`.
|
||||||
|
- [ ] **1.5 Widget delegates to shared helper**
|
||||||
|
- Files: `widgets/sources.py` (modify)
|
||||||
|
- Lines: ~-40
|
||||||
|
- Details: `SshTaskWidgetSource.fetch` calls `run_saved_task` instead of its
|
||||||
|
inline run+log block.
|
||||||
|
- [ ] **1.6 Add `list_service_task_runs` by task (if not present)**
|
||||||
|
- Files: `services/settings_store.py` (modify)
|
||||||
|
- Lines: ~10
|
||||||
|
- Details: Confirm `list_service_task_runs(task_id=...)` covers the tasks
|
||||||
|
router needs.
|
||||||
|
- [ ] **1.7 Update backend tests**
|
||||||
|
- Files: `backend/tests/test_jobs.py`, `test_api.py` (modify)
|
||||||
|
- Lines: ~60
|
||||||
|
- Details: Update task-run tests to the service model; cover override +
|
||||||
|
default + disabled-service paths.
|
||||||
|
- [ ] **1.8 Verify**
|
||||||
|
- Run: `cd backend && .venv/bin/ruff check . && PYTHONPATH=src .venv/bin/python -m pytest`
|
||||||
|
|
||||||
|
**Slice 1 total:** ~250 changed lines.
|
||||||
|
|
||||||
|
## Slice 2: Frontend — Actions page on services
|
||||||
|
|
||||||
|
**Goal:** Actions page targets ssh_tasks services; reads service_task_runs.
|
||||||
|
|
||||||
|
- [ ] **2.1 Update types**
|
||||||
|
- Files: `frontend/src/types/index.ts` (modify)
|
||||||
|
- Lines: ~15
|
||||||
|
- Details: `SavedTask` / `SavedTaskInput` `default_service_id`;
|
||||||
|
`SavedTaskRun` aligned to `service_task_runs`.
|
||||||
|
- [ ] **2.2 Update API client**
|
||||||
|
- Files: `frontend/src/api/client.ts` (modify)
|
||||||
|
- Lines: ~10
|
||||||
|
- Details: `runTask(taskId, serviceId?)` sends `service_id`.
|
||||||
|
- [ ] **2.3 Rewire Actions page**
|
||||||
|
- Files: `frontend/src/pages/Actions.tsx` (modify)
|
||||||
|
- Lines: ~120
|
||||||
|
- Details: Task editor "Default service" select lists ssh_tasks services via
|
||||||
|
`useServiceInstances("ssh_tasks")`; run dialog "Run on" selects an instance;
|
||||||
|
run history reads `service_task_runs`. Remove `useMonitoringSettings`.
|
||||||
|
- [ ] **2.4 Update Actions tests**
|
||||||
|
- Files: `frontend/src/pages/__tests__/Actions.test.tsx` (modify)
|
||||||
|
- Lines: ~30
|
||||||
|
- Details: Mock `useServiceInstances`; update fixtures.
|
||||||
|
- [ ] **2.5 Docs + changelog**
|
||||||
|
- Files: `docs/REQUIREMENTS.md`, `CHANGELOG.md` (modify)
|
||||||
|
- Lines: ~30
|
||||||
|
- Details: Saved-actions section: tasks target ssh_tasks services; local mode
|
||||||
|
dropped; breaking-upgrade note.
|
||||||
|
- [ ] **2.6 Verify**
|
||||||
|
- Run: `cd frontend && npm run lint && npm run build && npm run test`
|
||||||
|
|
||||||
|
**Slice 2 total:** ~200 changed lines.
|
||||||
|
|
||||||
|
## Integration and acceptance
|
||||||
|
|
||||||
|
- [ ] **3.1 Backend full test run** — `PYTHONPATH=src pytest`, all green.
|
||||||
|
- [ ] **3.2 Frontend full build/lint/test**.
|
||||||
|
- [ ] **3.3 Manual dev-stack check**:
|
||||||
|
- Create an ssh_tasks service; create a task with that default; run from
|
||||||
|
Actions; see the run in both the Actions history and the service page.
|
||||||
|
- Override the target at run time.
|
||||||
|
- SSH task widget uses the same history.
|
||||||
Reference in New Issue
Block a user