-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Add HTTP request workflow node frontend UI #2786
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
base: main
Are you sure you want to change the base?
Conversation
…schema - Add database migration for HTTP request block columns - Update workflow models to support HTTP request blocks - Add HTTP request block parsing and validation - Update workflow service to handle HTTP request execution - Add database model changes for HTTP request parameters
- Add HttpRequestNode component with full UI interface - Update workflow block icons and types - Add HTTP request node to workflow library panel - Update workflow types and YAML types for frontend - Add HTTP request node registration and exports
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.
Important
Looks good to me! 👍
Reviewed everything up to 51e5c8a in 1 minute and 51 seconds. Click for details.
- Reviewed
879
lines of code in9
files - Skipped
0
files when reviewing. - Skipped posting
4
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. skyvern-frontend/src/routes/workflows/editor/nodes/HttpRequestNode/HttpRequestNode.tsx:190
- Draft comment:
Using parseInt(e.target.value) || 30 may override a valid zero input—even if 0 is not desired, consider explicitly handling this edge case. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The comment is technically correct about the behavior of || with zero, but it's not relevant here because: 1. The input has min=1 constraint 2. Zero is not a valid timeout value for HTTP requests 3. The default of 30 seconds is reasonable 4. The suggested fix adds complexity without practical benefit The comment identifies a real programming pattern issue. Maybe there could be cases where we want to allow zero timeouts in the future? Zero timeouts don't make sense for HTTP requests, and the UI explicitly prevents values below 1. The current code is actually clearer and matches the business requirements better. Delete the comment. While technically accurate about || behavior, the suggestion would add unnecessary complexity given that zero is not a valid value here.
2. skyvern-frontend/src/routes/workflows/editor/nodes/HttpRequestNode/HttpRequestNode.tsx:74
- Draft comment:
In updateHeader(), consider validating the new header key (e.g. checking for empty string) to avoid unintended empty header names. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
3. skyvern-frontend/src/routes/workflows/editor/workflowEditorUtils.ts:532
- Draft comment:
For the HTTP request node conversion, inputMode is inferred based on curl_command. Consider clarifying or explicitly mapping inputMode to improve readability. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
4. skyvern-frontend/src/routes/workflows/editor/workflowEditorUtils.ts:1290
- Draft comment:
Ensure consistent transformation of property names (camelCase vs. snake_case) for the HTTP request node, such as mapping followRedirects (frontend) to follow_redirects (YAML). - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
Workflow ID: wflow_G4END60JidtLxDe1
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
Important
Looks good to me! 👍
Reviewed 2649083 in 1 minute and 5 seconds. Click for details.
- Reviewed
394
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
9
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. skyvern/forge/sdk/workflow/models/yaml.py:14
- Draft comment:
Consider adding detailed docstrings and a BaseConfig for these Pydantic models to clarify intended behavior and enforce consistent settings (e.g. extra='forbid'). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. skyvern/forge/sdk/workflow/models/yaml.py:20
- Draft comment:
The repeated use of 'type: ignore' for Literal fields is well‐documented; consider abstracting the common pattern if more parameter types are added. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. skyvern/forge/sdk/workflow/service.py:1696
- Draft comment:
The 'block_yaml_to_block' method spans many lines and uses a long if/elif chain. Consider refactoring it into a dispatcher (mapping block types to handler functions) to improve readability and ease future extension. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. skyvern/forge/sdk/workflow/service.py:330
- Draft comment:
Inside the workflow execution loop, there’s a call to refresh the workflow run status via 'app.DATABASE.get_workflow_run'. This could be a performance bottleneck if many blocks execute—consider caching or batching such database calls. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. skyvern/forge/sdk/workflow/service.py:521
- Draft comment:
Several try/except blocks catch generic Exception (e.g. around here). Catching more specific exceptions could help avoid masking unexpected issues. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. skyvern/forge/sdk/workflow/service.py:1520
- Draft comment:
In 'create_workflow_from_request', duplicate parameter keys and reserved keys are checked. Consider validating the uniqueness of block labels earlier and providing more descriptive error messages if duplicates are found. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. skyvern/forge/sdk/workflow/service.py:1908
- Draft comment:
In the ActionBlock branch, an exception is raised with a generic message ('empty action instruction'). Consider raising a custom exception with a more descriptive message to aid debugging. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. skyvern/forge/sdk/workflow/service.py:2117
- Draft comment:
Consider reviewing the 'persist_debug_artifacts' and related persist methods to ensure that awaiting asynchronous tasks (e.g. uploads) are bounded, and log a warning if cleanup takes unusually long. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. skyvern/forge/sdk/workflow/service.py:240
- Draft comment:
Many functions in WorkflowService lack detailed docstrings. Adding comprehensive function-level documentation will help future maintainers understand intent and usage. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_bDnhP0eOQq1CtlDV
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Screenshot? |
</Button> | ||
</div> | ||
<div className="space-y-2"> | ||
{Object.entries(inputs.headers).map(([key, value], index) => ( |
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.
A KeyValueInput
component was created to serve this exact purpose: managing custom headers. Can you re-use that?
You can grep for usage via KeyValueInput
. Here is an example: https://github.com/Skyvern-AI/skyvern-cloud/blob/main/skyvern-frontend/src/routes/workflows/workflowRun/WorkflowPostRunParameters.tsx#L155
Important
Add HTTP request workflow node to frontend UI and backend logic, enabling HTTP request configuration in workflows.
HttpRequestNode.tsx
toskyvern-frontend/src/routes/workflows/editor/nodes/HttpRequestNode/
for HTTP request configuration in workflows.WorkflowBlockIcon.tsx
to includeLinkBreak2Icon
for HTTP request nodes.nodeTypes
inindex.ts
to includehttpRequest
.HttpRequestBlock
type toworkflowTypes.ts
andworkflowYamlTypes.ts
.HttpRequestBlock
class toblock.py
for handling HTTP requests in workflows.service.py
to supportHttpRequestBlock
in workflow execution.7099cf601169_add_http_request_block_columns_to_.py
to add HTTP request columns toworkflow_run_blocks
.This description was created by
for 2649083. You can customize this summary. It will automatically update as commits are pushed.