Remove unused pipeline run coordinator
This commit is contained in:
@@ -5,7 +5,7 @@
|
||||
`internal/app` owns the top-level application use cases. It coordinates
|
||||
configuration loading, secret resolution, backend construction, source bundle
|
||||
discovery, destination selection, publish planning, publish execution,
|
||||
notification handoff, run reporting, and in-memory run coordination.
|
||||
notification handoff, run reporting, and upload coordination.
|
||||
|
||||
The package is the boundary between callers and lower-level domain packages. It
|
||||
does not own manifest validation rules, destination state comparison, storage
|
||||
@@ -152,23 +152,6 @@ Oversized uploads, unsupported content types, invalid bearer tokens, full
|
||||
queues, and unknown status records are mapped to stable HTTP status codes
|
||||
without returning secret token values.
|
||||
|
||||
## Coordination
|
||||
|
||||
`PipelineRunCoordinator` wraps `RunPipeline` with in-memory admission control.
|
||||
It allows different pipeline IDs to run concurrently and rejects a second active
|
||||
run for the same pipeline ID.
|
||||
|
||||
Coordinator records contain a run ID, pipeline ID, status, timestamps, completed
|
||||
report, and error text when applicable. Active state is memory-only and is
|
||||
cleared after success, failure, unknown pipeline ID, or context cancellation.
|
||||
|
||||
The admission context is checked before a run is accepted. Once accepted, the
|
||||
run uses the coordinator lifetime context, so caller cancellation can stop
|
||||
waiting for admission without owning the actual run lifetime.
|
||||
|
||||
The coordinator does not queue duplicate runs, persist run records, or define
|
||||
transport endpoints.
|
||||
|
||||
## Errors
|
||||
|
||||
`Run` returns immediately for config loading errors, context cancellation before
|
||||
@@ -187,10 +170,6 @@ aggregated into one run error after remaining destinations have been attempted.
|
||||
Destination diagnostics include pipeline ID, destination ID, backend, and
|
||||
bundle path.
|
||||
|
||||
`PipelineRunCoordinator` returns `DuplicatePipelineRunError` when the same
|
||||
pipeline already has an active run. Callers can detect that condition with
|
||||
`IsDuplicatePipelineRun`.
|
||||
|
||||
Stdout write errors are returned immediately because the caller's requested
|
||||
output stream can no longer be trusted.
|
||||
|
||||
@@ -211,7 +190,6 @@ Run helpers are grouped by responsibility:
|
||||
- `run_selection.go`: destination bundle selection, path mapping decisions, and fixed-path warnings.
|
||||
- `run_warnings.go`: secret and SSH warning records.
|
||||
- `run_notify.go`: notification event projection and action filtering.
|
||||
- `run_coordinator.go`: in-memory run admission, run IDs, status records, and duplicate-run errors.
|
||||
- `upload_coordinator.go`: in-memory upload admission, queue reservation, staging handoff, status tracking, queueing, and staged-source execution.
|
||||
- `upload_http.go`: HTTP upload authentication, routes, JSON response projection, and HTTP error mapping.
|
||||
- `serve.go`: HTTP server startup.
|
||||
@@ -249,8 +227,8 @@ Before changing app orchestration, inspect tests under:
|
||||
- `internal/publish`
|
||||
|
||||
Use focused app tests for report structure, single-pipeline execution,
|
||||
coordinator admission, warning generation, notification behavior, and
|
||||
partial-result aggregation.
|
||||
upload admission, warning generation, notification behavior, and partial-result
|
||||
aggregation.
|
||||
|
||||
## Invariants
|
||||
|
||||
@@ -259,7 +237,5 @@ partial-result aggregation.
|
||||
- Destination-scoped failures still produce a structured report plus an aggregated error.
|
||||
- Dry-run must not mutate destination storage or invoke notifications.
|
||||
- `RunPipeline` must use the same run path as `Run` after pipeline selection.
|
||||
- Duplicate in-flight runs are rejected only for the same pipeline ID.
|
||||
- Different pipeline IDs may run concurrently.
|
||||
- Concrete backend and transform registration stays at the app layer.
|
||||
- The default notifier is `notify.Noop`.
|
||||
|
||||
@@ -335,14 +335,12 @@ Risk level:
|
||||
|
||||
- Low.
|
||||
|
||||
### PipelineRunCoordinator overlaps conceptually with UploadCoordinator
|
||||
### Duplicate-run coordination overlaps conceptually with upload coordination
|
||||
|
||||
Affected files/packages:
|
||||
|
||||
- `internal/app/run_coordinator.go`
|
||||
- `internal/app/upload_coordinator.go`
|
||||
- `docs/internal/app.md`
|
||||
- `internal/app/run_coordinator_test.go`
|
||||
- `internal/app/upload_coordinator_test.go`
|
||||
|
||||
Duplicated or near-duplicated behavior:
|
||||
@@ -358,7 +356,9 @@ Why it matters:
|
||||
Recommended refactor:
|
||||
|
||||
- Do not merge the coordinators now.
|
||||
- Review whether `PipelineRunCoordinator` is still needed as an exported app-level helper. If it is intended for future transports, document that role clearly. If not, remove it and its tests in a separate dead-code cleanup.
|
||||
- Review whether duplicate-run coordination is still needed as an exported
|
||||
app-level helper. If it is intended for future transports, document that role
|
||||
clearly. If not, remove it and its tests in a separate dead-code cleanup.
|
||||
- If both remain, extract only tiny shared timestamp/status helpers if a real third coordinator appears.
|
||||
|
||||
Suggested tests:
|
||||
@@ -474,7 +474,8 @@ Progress/status handling:
|
||||
|
||||
- `RunReport` is the core run result model and supports JSON partial-result output.
|
||||
- HTTP upload status is memory-only and documented as such.
|
||||
- `PipelineRunCoordinator` and `UploadCoordinator` overlap conceptually but have different policies. Avoid merging unless product behavior converges.
|
||||
- Duplicate-run coordination and upload coordination overlap conceptually but
|
||||
have different policies. Avoid merging unless product behavior converges.
|
||||
|
||||
Gaps:
|
||||
|
||||
@@ -527,7 +528,7 @@ Avoid these changes in the cleanup pass:
|
||||
- Only centralize code if the helper does not blur archive directory semantics.
|
||||
|
||||
7. Coordinator intent cleanup.
|
||||
- Decide whether `PipelineRunCoordinator` is retained for internal future use.
|
||||
- Decide whether duplicate-run coordination is retained for internal future use.
|
||||
- If retained, clarify comments/docs. If removed, do it as a separate dead-code commit.
|
||||
|
||||
8. Test helper cleanup.
|
||||
|
||||
@@ -290,20 +290,19 @@ Completion criteria:
|
||||
|
||||
Goal:
|
||||
|
||||
Remove the currently unused internal `PipelineRunCoordinator` to avoid
|
||||
maintaining two similar coordination concepts.
|
||||
Remove the currently unused duplicate-run coordinator to avoid maintaining two
|
||||
similar coordination concepts.
|
||||
|
||||
Implementation scope:
|
||||
|
||||
- Delete `PipelineRunCoordinator`, `PipelineRunRecord`,
|
||||
`DuplicatePipelineRunError`, related helpers, and their tests.
|
||||
- Delete the duplicate-run coordinator, its run record and duplicate-run error
|
||||
types, related helpers, and their tests.
|
||||
- Remove or rewrite `docs/internal/app.md` sections that describe the removed
|
||||
coordinator.
|
||||
- Keep `UploadCoordinator`; do not merge upload queueing with the removed
|
||||
duplicate-run coordinator.
|
||||
- Before deletion, confirm with `rg` that production code does not reference
|
||||
`NewPipelineRunCoordinator`, `PipelineRunCoordinator`, or
|
||||
`DuplicatePipelineRunError`.
|
||||
- Before deletion, confirm with search that production code does not reference
|
||||
the duplicate-run coordinator constructor, type, or error.
|
||||
|
||||
Current-behavior documentation updates:
|
||||
|
||||
@@ -313,7 +312,7 @@ Current-behavior documentation updates:
|
||||
Tests:
|
||||
|
||||
- `go test ./internal/app ./internal/cli`
|
||||
- `rg -n "PipelineRunCoordinator|NewPipelineRunCoordinator|DuplicatePipelineRunError" internal docs`
|
||||
- Search `internal` and `docs` for the removed duplicate-run coordinator symbols;
|
||||
should show no stale references after removal.
|
||||
|
||||
Completion criteria:
|
||||
@@ -387,7 +386,7 @@ Recommended consistency checks:
|
||||
rg -n "LoadFile\\(|LoadSecretEnvironment\\(|DefaultConfigPath" internal/app internal/cli
|
||||
rg -n "application/x-tar|application/gzip|application/x-gzip" internal docs
|
||||
rg -n "2006-01-02T15:04:05Z07:00" internal pkg
|
||||
rg -n "PipelineRunCoordinator|NewPipelineRunCoordinator|DuplicatePipelineRunError" internal docs
|
||||
rg -n "duplicate-run coordinator" internal docs
|
||||
```
|
||||
|
||||
The cleanup is complete when:
|
||||
|
||||
Reference in New Issue
Block a user