-
-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add POST /api/v1/workflow/{workflowId}/run endpoint
#241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@UlisesGascon has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe changes introduce a new POST endpoint to execute workflows via the API, update the workflow retrieval and execution logic to use a new details function, and enhance test coverage for workflow execution scenarios. The API specification and importer function signatures are updated to reflect these changes, supporting both file path and direct data input. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Server
participant Workflows_Module
Client->>API_Server: POST /api/v1/workflow/{id}/run (with data)
API_Server->>Workflows_Module: getWorkflowsDetails()
API_Server->>API_Server: Validate workflow ID
alt Workflow found
API_Server->>Workflows_Module: Run workflow function with data (with timeout)
alt Workflow completes
API_Server-->>Client: 202 Accepted (status: completed, workflow details)
else Workflow fails
API_Server-->>Client: 500 Internal Server Error (status: failed, error)
end
else Workflow not found
API_Server-->>Client: 404 Not Found (error message)
end
Possibly related PRs
Poem
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/cli/workflows.js (1)
37-48: Avoid shadowing & favour expressive namingThe local constant
workflowshides the public-facing variable you’re about to return. A more explicit name (e.g.workflowsMap) prevents any mental gymnastics when reading the code and avoids accidental confusion with the top-levelcommandList.- const workflows = {} + const workflowsMap = {} const workflowsList = [] commandList.forEach((workflow) => { const workflowName = _.kebabCase(workflow.name) workflowsList.push({ id: workflowName, description: workflow.description }) - workflows[workflowName] = { + workflowsMap[workflowName] = { description: workflow.description, workflow: workflow.workflow } }) - return { workflows, workflowsList } + return { workflows: workflowsMap, workflowsList }__tests__/httpServer/apiV1.test.js (1)
6-21: Duplicate mocks hide errors
mockWorkflowFnis declared twice – once at file scope (line 6) and again inside the POST-workflowbeforeEach(line 153).
If a test outside thatdescribeblock unexpectedly relies on the outer mock, its call-count assertions will silently break.Delete the top-level declaration and keep the per-suite version (or hoist the inner one).
This keeps mock state local and predictable.src/httpServer/routers/apiV1.js (1)
76-105: OpenAPI mismatch & double lookup
getWorkflowsDetails()is invoked once to buildworkflows, then again insiderunWorkflow(). Pass the resolved map intorunWorkflow()(or curry the function) to avoid redundant work.The error payload uses
workflow.descriptionwhen the workflow might beundefined; this is covered by the ternary but can be simplified by returning early.src/httpServer/swagger/api-v1.yml (1)
81-87: Path parameter naming driftExpress route is
/workflow/:id/run(paramid) but the spec defines{workflowId}.
Keep them consistent to avoid confusion in autogenerated clients.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
__tests__/httpServer/apiV1.test.js(3 hunks)src/cli/workflows.js(3 hunks)src/httpServer/routers/apiV1.js(2 hunks)src/httpServer/swagger/api-v1.yml(1 hunks)src/importers/index.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/cli/workflows.js (3)
__tests__/httpServer/apiV1.test.js (1)
getWorkflowsDetails(31-31)src/cli/index.js (1)
workflows(2-2)visionboard.js (1)
workflow(35-37)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright Tests
🔇 Additional comments (1)
__tests__/httpServer/apiV1.test.js (1)
148-172: Consider testing the timeout branchA TODO already exists, but note that
runWorkflow()rejects after 30 s by default.
Usejest.useFakeTimers()and advance the timers to assert the 500 path without waiting in real time.
83382a9 to
6b025b9
Compare
6b025b9 to
e8a63be
Compare
Related #216
Summary by CodeRabbit
New Features
Improvements
Tests