feat: new task endpoints #249
Conversation
jason-lynch
force-pushed
the
feat/PLAT-347/add-scope-to-tasks
branch
from
9ff3ccb to
83955e8
Compare
jason-lynch
force-pushed
the
feat/PLAT-347/new-task-endpoints
branch
from
4c7d9b7 to
f43d9aa
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis change introduces host-scoped task management endpoints alongside a cross-scope task listing endpoint to the control-plane API. New capabilities include listing and retrieving host-specific tasks, accessing host task logs, and querying tasks across database or host scopes. Client interfaces, single and multi-server implementations, and server-side API handlers are added. Task-related helper methods are renamed to clarify database versus host scope distinction. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jason-lynch
force-pushed
the
feat/PLAT-347/new-task-endpoints
branch
2 times, most recently
from
85c8902 to
f5b0ba8
Compare
jason-lynch
force-pushed
the
feat/PLAT-347/add-scope-to-tasks
branch
2 times, most recently
from
7391dab to
53678c0
Compare
jason-lynch
force-pushed
the
feat/PLAT-347/new-task-endpoints
branch
from
f5b0ba8 to
e2c5ecc
Compare
jason-lynch
force-pushed
the
feat/PLAT-347/add-scope-to-tasks
branch
from
53678c0 to
8619be9
Compare
jason-lynch
force-pushed
the
feat/PLAT-347/new-task-endpoints
branch
2 times, most recently
from
ff7f98c to
57c9324
Compare
jason-lynch
force-pushed
the
feat/PLAT-347/add-scope-to-tasks
branch
from
8619be9 to
baedea3
Compare
jason-lynch
force-pushed
the
feat/PLAT-347/new-task-endpoints
branch
from
57c9324 to
bdd4154
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@e2e/cancel_task_test.go`:
- Around line 64-67: The test calls fixture.Client.WaitForDatabaseTask and
immediately uses final_task in assertions without checking err; modify the test
around the WaitForDatabaseTask call (the variables final_task, err, and the
GetDatabaseTaskPayload using cancelation_task.TaskID) to assert or fail
immediately when err != nil (e.g., t.Fatalf or require.NoError) so the test
stops on waiter failures before accessing final_task.
In `@hack/dev-env.zsh`:
- Around line 548-555: The code treats JSON null strings as valid because jq -r
returns "null"; update the three jq extractions (scope, entity_id, task_id) to
convert JSON null to an empty string (e.g., use the jq "// empty" or equivalent)
so that nulls fail the existing -z check, or alternatively add a check that
treats the literal "null" as empty (test for == "null"). Change the lines that
set scope, entity_id, and task_id (the variables named scope, entity_id,
task_id) to use that null-to-empty handling so the subsequent if [[ -z ... ]]
correctly rejects missing fields.
- Around line 72-96: The scope selection calls jq on a plain string causing
parse errors; in _choose-scope, stop piping scope_choice through jq—set scope to
the chosen string (trim whitespace/newlines) and log "using scope ${scope}"
instead of trying to parse JSON. In _choose-host, change the log message from
"using database ${host_id}" to "using host ${host_id}" (host_id is correctly
extracted with jq -r '.id') so the label matches the selection.
🧹 Nitpick comments (1)
client/single.go (1)
246-322: Consider extracting shared logic to reduce duplication.
FollowDatabaseTaskandFollowHostTaskshare nearly identical structure—the only differences are the payload types and theGetXxxTaskLogmethod called. While this duplication is acceptable for now, consider a similar callback-based refactor (likewaitForTask) if more scope-specific follow methods are added in the future.♻️ Possible future refactor using generics
// A generic follow helper could look like: func followTask[P any]( ctx context.Context, req P, cloneReq func(P, *string) P, getLog func(context.Context, P) (*api.TaskLog, error), handler func(e *api.TaskLogEntry), ) error { // ... shared polling logic }This is optional and could be deferred until there's a third scope or more complexity.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
api/apiv1/gen/control_plane/client.gois excluded by!**/gen/**api/apiv1/gen/control_plane/endpoints.gois excluded by!**/gen/**api/apiv1/gen/control_plane/service.gois excluded by!**/gen/**api/apiv1/gen/http/cli/control_plane/cli.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/client/cli.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/client/client.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/client/encode_decode.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/client/paths.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/client/types.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/server/encode_decode.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/server/paths.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/server/server.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/server/types.gois excluded by!**/gen/**api/apiv1/gen/http/openapi.jsonis excluded by!**/gen/**api/apiv1/gen/http/openapi.yamlis excluded by!**/gen/**api/apiv1/gen/http/openapi3.jsonis excluded by!**/gen/**api/apiv1/gen/http/openapi3.yamlis excluded by!**/gen/**
📒 Files selected for processing (13)
api/apiv1/design/api.goapi/apiv1/design/task.gochanges/unreleased/Added-20260114-162946.yamlclient/interface.goclient/multi.goclient/single.godocs/using/tasks-logs.mde2e/cancel_task_test.goe2e/database_test.gohack/dev-env.zshserver/internal/api/apiv1/convert.goserver/internal/api/apiv1/post_init_handlers.goserver/internal/api/apiv1/pre_init_handlers.go
🧰 Additional context used
📓 Path-based instructions (4)
server/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
server/**/*.go: Usesamber/doinjector for dependency injection; each package should have aProvide()function that registers dependencies
Use structured JSON logging with zerolog throughout the codebase, with pretty-printing enabled in dev mode
Domain-specific errors should be defined in each package; API errors should be mapped to HTTP status codes via Goa
Files:
server/internal/api/apiv1/convert.goserver/internal/api/apiv1/post_init_handlers.goserver/internal/api/apiv1/pre_init_handlers.go
server/internal/api/apiv1/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Implement generated Goa service interface methods in
server/internal/api/apiv1/after regenerating code
Files:
server/internal/api/apiv1/convert.goserver/internal/api/apiv1/post_init_handlers.goserver/internal/api/apiv1/pre_init_handlers.go
e2e/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should use build tag
//go:build e2e_testand place test fixtures ine2e/fixtures/
Files:
e2e/database_test.goe2e/cancel_task_test.go
api/apiv1/design/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
API endpoints should be defined using Goa's DSL in
api/apiv1/design/with separate files for domain-specific types (database.go, host.go, cluster.go), then regenerated withmake -C api generate
Files:
api/apiv1/design/api.goapi/apiv1/design/task.go
🧠 Learnings (1)
📚 Learning: 2026-01-14T16:43:14.333Z
Learnt from: CR
Repo: pgEdge/control-plane PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T16:43:14.333Z
Learning: Applies to api/apiv1/design/**/*.go : API endpoints should be defined using Goa's DSL in `api/apiv1/design/` with separate files for domain-specific types (database.go, host.go, cluster.go), then regenerated with `make -C api generate`
Applied to files:
api/apiv1/design/api.goapi/apiv1/design/task.goclient/single.go
🧬 Code graph analysis (8)
server/internal/api/apiv1/convert.go (4)
api/apiv1/gen/control_plane/service.go (3)
ListHostTasksPayload(691-700)GetHostTaskLogPayload(520-529)ListTasksPayload(717-728)server/internal/task/task_store.go (2)
TaskListOptions(55-64)SortOrder(44-44)server/internal/task/task_log_store.go (1)
TaskLogOptions(73-76)server/internal/task/task.go (3)
Scope(13-13)ScopeDatabase(20-20)ScopeHost(21-21)
e2e/database_test.go (1)
api/apiv1/gen/control_plane/service.go (1)
GetDatabaseTaskPayload(504-509)
server/internal/api/apiv1/post_init_handlers.go (5)
api/apiv1/gen/control_plane/service.go (4)
ListHostTasksPayload(691-700)ListHostTasksResponse(704-706)Task(938-965)TaskLog(969-984)api/apiv1/design/task.go (3)
ListHostTasksResponse(314-331)Task(7-79)TaskLog(101-262)server/internal/task/task.go (4)
Task(67-83)TaskLog(246-253)Status(45-45)Scope(13-13)server/internal/api/apiv1/errors.go (1)
ErrInvalidTaskID(33-33)server/internal/task/task_store.go (2)
TaskListOptions(55-64)SortOrder(44-44)
e2e/cancel_task_test.go (2)
client/interface.go (1)
Client(9-44)api/apiv1/gen/control_plane/service.go (1)
GetDatabaseTaskPayload(504-509)
server/internal/api/apiv1/pre_init_handlers.go (3)
api/apiv1/gen/control_plane/service.go (8)
ListHostTasksPayload(691-700)ListHostTasksResponse(704-706)GetHostTaskPayload(533-538)Task(938-965)GetHostTaskLogPayload(520-529)TaskLog(969-984)ListTasksPayload(717-728)ListTasksResponse(732-734)server/internal/api/apiv1/errors.go (1)
ErrUninitialized(32-32)server/internal/task/task.go (2)
Task(67-83)TaskLog(246-253)
client/multi.go (3)
api/apiv1/gen/control_plane/service.go (9)
ListTasksPayload(717-728)ListTasksResponse(732-734)ListHostTasksPayload(691-700)ListHostTasksResponse(704-706)GetHostTaskPayload(533-538)Task(938-965)GetHostTaskLogPayload(520-529)TaskLog(969-984)TaskLogEntry(986-993)api/apiv1/design/task.go (5)
ListTasksResponse(333-361)ListHostTasksResponse(314-331)Task(7-79)TaskLog(101-262)TaskLogEntry(81-99)server/internal/task/task.go (2)
Task(67-83)TaskLog(246-253)
api/apiv1/design/task.go (2)
server/internal/task/task.go (2)
Type(24-24)Task(67-83)api/apiv1/gen/control_plane/service.go (1)
Task(938-965)
client/single.go (5)
client/enums.go (3)
TaskStatusCompleted(86-86)TaskStatusCanceled(90-90)TaskStatusFailed(87-87)server/internal/resource/resource.go (1)
Context(84-88)api/apiv1/gen/control_plane/service.go (8)
ListTasksPayload(717-728)ListTasksResponse(732-734)ListHostTasksResponse(704-706)GetHostTaskPayload(533-538)Task(938-965)GetHostTaskLogPayload(520-529)TaskLog(969-984)TaskLogEntry(986-993)api/apiv1/design/task.go (5)
ListTasksResponse(333-361)ListHostTasksResponse(314-331)Task(7-79)TaskLog(101-262)TaskLogEntry(81-99)server/internal/task/task.go (3)
Task(67-83)TaskLog(246-253)Status(45-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (18)
server/internal/api/apiv1/convert.go (1)
795-867: Nice helper coverage for host/generic task options.
Validation and parsing mirror the database helpers and keep behavior consistent.api/apiv1/design/task.go (1)
313-361: Clear response types for new task listings.
Examples are consistent with the Task schema and make the intent obvious.api/apiv1/design/api.go (1)
529-678: Endpoints align with existing task API patterns.
Payloads, result types, and error models look consistent.server/internal/api/apiv1/post_init_handlers.go (2)
1087-1152: Host task handlers follow established database-task patterns.
Looks consistent in validation and data retrieval.
1154-1195: No changes needed — the endpoint correctly returns tasks across all scopes whenscopeis omitted.When
req.Scopeis nil,taskListOptionsFromGenericreturns an empty scope string. The storage layer'sEntityPrefixfunction with an empty scope produces/tasks_v2/(empty strings are filtered by Go'spath.Join), which serves as a prefix for all scope-specific tasks. The etcd range query matches all tasks under this prefix, returning results from bothdatabaseandhostscopes. This behavior is intentional and documented in the Goa API design (seeListTasksResponseexample showing mixed-scope results).Likely an incorrect or invalid review comment.
server/internal/api/apiv1/pre_init_handlers.go (1)
274-288: Pre-init stubs are consistent with existing behavior.
ReturningErrUninitializedkeeps the contract clear.changes/unreleased/Added-20260114-162946.yaml (1)
1-3: Changelog entry looks good.Clear and concise summary of the new API endpoints.
e2e/database_test.go (1)
339-343: Waiter update is correct.The database-scoped waiter matches the new API surface.
docs/using/tasks-logs.md (1)
142-151: No action needed. The endpoint path documented at/v1/hosts/{host_id}/tasks/{task_id}/logsmatches the actual API implementation in the codebase across all route handlers, service definitions, and OpenAPI specs. Users will not receive 404s.client/interface.go (2)
25-31: LGTM! Well-structured API method additions.The new task endpoints (
ListTasks,ListHostTasks,GetHostTask,GetHostTaskLog) are correctly positioned in the interface and use the appropriate generated payload/response types, aligning with the PR objective of adding host-scoped task management.
40-43: Good separation of database vs. host task helpers.The renamed
WaitForDatabaseTask/FollowDatabaseTaskand newWaitForHostTask/FollowHostTaskmethods provide clear scope-specific APIs, improving clarity over the previous genericWaitForTask/FollowTasknames.client/multi.go (3)
210-216: LGTM! Consistent pass-through implementation.The
ListTasksmethod follows the established pattern of delegating to a live server, consistent with other read operations in this file.
242-264: LGTM! Host task methods follow the established pattern.
ListHostTasks,GetHostTask, andGetHostTaskLogare implemented consistently with the existing database task methods, correctly delegating to the live server.
313-343: LGTM! Wait and follow helpers correctly delegate to SingleServerClient.The renamed
WaitForDatabaseTask/FollowDatabaseTaskand newWaitForHostTask/FollowHostTaskmethods properly delegate to the underlyingSingleServerClientimplementation where the actual polling logic resides.client/single.go (4)
18-22: LGTM! Terminal status map correctly excludes transitional states.The
taskEndedmap correctly identifiescompleted,canceled, andfailedas terminal states. Notably,cancelingis correctly excluded since it's a transitional state (the task is still in progress of being canceled).
72-75: LGTM! New endpoint wiring is complete.All four new endpoints (
ListHostTasks,GetHostTask,GetHostTaskLog,ListTasks) are properly wired to the generated client methods.
145-178: LGTM! New API methods follow established patterns.
ListTasks,ListHostTasks,GetHostTask, andGetHostTaskLogare implemented consistently with the existing database task methods, properly usingtranslateErrfor error handling.
195-229: Good refactor extracting common polling logic.The
waitForTaskhelper with agetTaskcallback cleanly abstracts the polling logic, allowingWaitForDatabaseTaskandWaitForHostTaskto share the implementation while remaining type-safe through their specific payload types.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
jason-lynch
force-pushed
the
feat/PLAT-347/new-task-endpoints
branch
from
bdd4154 to
e77c094
Compare
jason-lynch
force-pushed
the
feat/PLAT-347/add-scope-to-tasks
branch
from
baedea3 to
1ad0c8b
Compare
Adds endpoints to get/list host tasks and to get host task logs. PLAT-347
Adds an endpoint to list tasks across all entities. PLAT-347
Updates `cp-follow` task to support host-scoped tasks. PLAT-347
Updates `task-logs.md` document with usage examples for the new endpoints and adds a changelog entry for the new endpoints. PLAT-347
jason-lynch
force-pushed
the
feat/PLAT-347/new-task-endpoints
branch
from
e77c094 to
61a0c96
Compare
Summary
Adds new endpoints to track host-scoped tasks:
list-host-tasksget-host-taskget-host-task-logAs well as a new endpoint to list tasks across all scopes:
list-tasksTesting
Nothing produces host tasks in this PR (will be added in a subsequent PR), but you should be able to see database tasks with the
list-tasksendpoint.PLAT-347
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.