Improve worker management admin route #2032
Conversation
|
WalkthroughThe changes update documentation for worker group management in the supervisor app and significantly extend the API endpoint logic for managing worker groups. The API now supports explicit creation, assignment, and removal of default worker groups for projects, with more granular request parameters, improved error handling, and clearer control flow. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant DB
Client->>API: POST /admin.api.v1.workers (create group)
API->>DB: Check if worker group exists
alt Group does not exist
API->>DB: Create new worker group
alt makeDefaultForProject is true
API->>DB: Set new group as default for project
API-->>Client: Success (group, no token)
else makeDefaultForProject is false
API-->>Client: Success (group, token)
end
else Group exists
alt makeDefaultForProject is true
API->>DB: Set existing group as default for project
API-->>Client: Success (group, no token)
else
API-->>Client: Error (group already exists)
end
end
Client->>API: POST /admin.api.v1.workers (removeDefaultFromProject)
API->>DB: Remove default worker group from project
API-->>Client: Success or Error
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
apps/supervisor/README.md (1)
48-106: Consider collapsing the duplicate cURL snippets & emphasise required flagsGreat to see explicit examples, but the three nearly-identical cURL blocks differ only in JSON payload. Collapsing them into a single block with inline comments (
# create,# set default,# remove default) reduces duplication and makes future edits less error-prone.apps/webapp/app/routes/admin.api.v1.workers.ts (4)
41-43:rawBody ?? {}is redundant
request.json()already returns an object;?? {}is never triggered.
Very minor, but removing it keeps parsing intent clearer.
108-116: Error message could leak existence of worker groupReturning
workerGroupdetails in the 400 response (worker group already exists) may inadvertently expose internal IDs to callers.
Consider omitting the full object or guarding behind an additional admin-only flag.
143-149: Top-level catch swallows non-JSON parsing errorsA malformed
Content-Type(text/plain) throws beforerawBodyassignment and lands here, returning 400 “unknown error”.
Explicitly validaterequest.headers.get("content-type")?.includes("application/json")first, returning 415 Unsupported Media Type.
196-244: Use a single DB transaction for default-update path
setWorkerGroupAsDefaultForProject()reads, then updates in two separate queries.
Wrap both in a singleprisma.$transactionto prevent a race where the project is deleted between calls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/supervisor/README.md(2 hunks)apps/webapp/app/routes/admin.api.v1.workers.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/supervisor/README.md (1)
22-23: Mismatch between documentation and implementation regardingtokenemissionThe README specifies that no token will be returned when an existing worker group is attached as a project-default, yet the updated endpoint (
apps/webapp/app/routes/admin.api.v1.workers.ts, lines 100-105) still includestokenin the JSON response whenmakeDefaultForProjectistrueand the group is newly created.
Please either
- update the endpoint to omit
tokenin that control-flow branch, or- amend the README to reflect the current behaviour.
Keeping docs and code in sync avoids confusion for operators automating their deployments.
It's now more easily possible to do the following:
Existing worker groups are handled gracefully, but no tokens will be returned for those as we only store hashes.
Summary by CodeRabbit
New Features
Documentation