-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Runware: new action component #14380
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
@jcortes is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughA new module has been added to the Runware component, enabling task requests to the Runware API. This includes defining constants for API interaction and enhancing the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 6
🧹 Outside diff range and nitpick comments (3)
components/runware/package.json (1)
15-18
: Dependencies added correctly.The addition of "@pipedream/platform" and "uuid" dependencies is appropriate for the new features being implemented. However, consider the following suggestion:
Consider pinning the "uuid" dependency to a specific version (e.g., "10.0.0" instead of "^10.0.0") to ensure consistent behavior across different environments.
components/runware/common/constants.mjs (2)
1-2
: LGTM! Consider environment-specific configuration.The BASE_URL and VERSION_PATH constants are correctly defined and align with the Runware API mentioned in the PR objectives.
Consider using environment variables for these values to allow for easier configuration across different environments (e.g., development, staging, production).
4-29
: LGTM! Consider using an enum-like structure for better maintainability.The TASK_TYPE object is well-structured and aligns with the image inference capabilities mentioned in the PR objectives. The naming conventions are consistent and clear.
For improved maintainability and type safety, consider using an enum-like structure. Here's an example using Object.freeze():
const TASK_TYPE = Object.freeze({ IMAGE_INFERENCE: { value: "imageInference", label: "Image Inference", }, // ... other task types ... });This approach prevents accidental modifications to the TASK_TYPE object.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
- components/runware/actions/request-task/request-task.mjs (1 hunks)
- components/runware/common/constants.mjs (1 hunks)
- components/runware/package.json (2 hunks)
- components/runware/runware.app.mjs (1 hunks)
🧰 Additional context used
🔇 Additional comments (5)
components/runware/package.json (2)
3-3
: Version update looks good.The version bump from 0.0.1 to 0.1.0 correctly reflects the addition of new features (Runware API integration) without breaking changes, adhering to semantic versioning principles.
Line range hint
1-19
: package.json structure and content are well-formed.The file contains all necessary fields for a Node.js package, including appropriate metadata and configuration. The public access setting in "publishConfig" is correct for a package in the @pipedream organization.
components/runware/common/constants.mjs (2)
31-35
: LGTM! Export structure is clean and appropriate.The default export includes all defined constants, which is correct and allows for easy import and use in other parts of the application.
1-35
: Overall, excellent implementation of constants for Runware API.This new file effectively defines the necessary constants for interacting with the Runware API, including various task types for image processing. The structure is clean, consistent, and aligns well with the PR objectives. The minor suggestions provided earlier are for potential improvements in flexibility and maintainability, but the current implementation is solid and ready for use.
components/runware/runware.app.mjs (1)
148-150
: Verify the error handling in themakeRequest
method.Currently, the code checks for
response.errors
, but Axios typically throws errors for non-2xx responses. Ensure that this check accurately captures API errors as intended.Would you like assistance in updating the error handling to correctly handle API and network errors?
f0aae2f
to
1e68e23
Compare
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: 2
🧹 Outside diff range and nitpick comments (2)
components/runware/actions/request-task/request-task.mjs (1)
10-120
: LGTM: Props are well-defined using app instance.The props are comprehensively defined using propDefinitions from the app instance, which promotes consistency and reusability. The wide range of parameters aligns well with the Runware API's capabilities as described in the PR objectives.
Consider adding comments or grouping related props to improve readability, especially for developers who might not be familiar with all the Runware API parameters.
components/runware/runware.app.mjs (1)
140-157
: Consider enhancing error handling in makeRequest method.The
makeRequest
method correctly checks for errors in the response. However, consider providing more context in the error message to aid in debugging. For example:if (response.errors) { throw new Error(`Runware API error: ${JSON.stringify(response.errors)}`); }This change would make it clearer that the error is coming from the Runware API.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
- components/runware/actions/request-task/request-task.mjs (1 hunks)
- components/runware/common/constants.mjs (1 hunks)
- components/runware/package.json (2 hunks)
- components/runware/runware.app.mjs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/runware/common/constants.mjs
- components/runware/package.json
🧰 Additional context used
🔇 Additional comments (6)
components/runware/actions/request-task/request-task.mjs (2)
1-9
: LGTM: Imports and metadata are well-defined.The imports and metadata for the Runware API task request action are appropriately set up. The use of
uuid
for generating unique task identifiers and the clear description with a link to the documentation are good practices.
176-179
: LGTM: Summary export and response return.The summary export and full response return are good practices, providing useful information to the user and allowing for further processing if needed.
As mentioned in a previous review, there's a typo in the summary message. "reuested" should be "requested". Please apply the following fix:
- $.export("$summary", `Successfully reuested task with UUID \`${response.data[0].taskUUID}\`.`); + $.export("$summary", `Successfully requested task with UUID \`${response.data[0].taskUUID}\`.`);components/runware/runware.app.mjs (4)
1-6
: LGTM: Imports and app definition are appropriate.The imports and app definition are correctly set up for a Runware API integration.
7-128
: LGTM: Comprehensive property definitions for Runware API.The property definitions are extensive, well-documented, and cover a wide range of parameters for the Runware API. Good job on providing detailed descriptions and appropriate options for each property.
130-139
: LGTM: URL and header construction methods are well-implemented.The
getUrl
andgetHeaders
methods are correctly implemented to construct the API URL and add necessary headers, including authentication.
14-34
: 🛠️ Refactor suggestionConsider defining options as constants for outputType and outputFormat.
As suggested in a previous review, consider defining the options for
outputType
andoutputFormat
as constants inconstants.mjs
to improve maintainability and ensure consistency across the codebase.
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.
Hi @jcortes, LGTM! Ready for QA!
1e68e23
to
09e441f
Compare
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
🧹 Outside diff range and nitpick comments (4)
components/runware/runware.app.mjs (4)
54-58
: AddminLength
andmaxLength
validations topositivePrompt
To enforce the length constraints mentioned in the description (between 4 and 2000 characters), add
minLength
andmaxLength
validations.Apply this diff to add validation:
positivePrompt: { type: "string", label: "Positive Prompt", description: "A positive prompt is a text instruction to guide the model on generating the image. It is usually a sentence or a paragraph that provides positive guidance for the task. This parameter is essential to shape the desired results. For example, if the positive prompt is `dragon drinking coffee`, the model will generate an image of a dragon drinking coffee. The more detailed the prompt, the more accurate the results. The length of the prompt must be between 4 and 2000 characters.", + minLength: 4, + maxLength: 2000, },
59-64
: AddminLength
andmaxLength
validations tonegativePrompt
Similarly, to enforce length constraints, add
minLength
andmaxLength
validations to thenegativePrompt
property.Apply this diff:
negativePrompt: { type: "string", label: "Negative Prompt", description: "A negative prompt is a text instruction to guide the model on generating the image. It is usually a sentence or a paragraph that provides negative guidance for the task. This parameter helps to avoid certain undesired results. For example, if the negative prompt is `red dragon, cup`, the model will follow the positive prompt but will avoid generating an image of a red dragon or including a cup. The more detailed the prompt, the more accurate the results. The length of the prompt must be between 4 and 2000 characters.", optional: true, + minLength: 4, + maxLength: 2000, },
65-70
: Implement conditional requirement forseedImage
based ontaskType
When
taskType
isImage-to-Image
,Inpainting
, orOutpainting
, theseedImage
parameter is required. Consider adding validation to enforce this conditional requirement.Would you like assistance in implementing this validation?
71-76
: Implement conditional requirement formaskImage
based ontaskType
Similarly, when performing
Inpainting
orOutpainting
, themaskImage
parameter is required. Adding validation will ensure users provide necessary inputs based on the selectedtaskType
.Would you like help in adding this validation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
- components/runware/actions/request-task/request-task.mjs (1 hunks)
- components/runware/common/constants.mjs (1 hunks)
- components/runware/package.json (2 hunks)
- components/runware/runware.app.mjs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/runware/actions/request-task/request-task.mjs
- components/runware/common/constants.mjs
- components/runware/package.json
🧰 Additional context used
🔇 Additional comments (3)
components/runware/runware.app.mjs (3)
19-23
: Previous suggestion on using constants foroutputType
options is still applicableDefining
outputType
options using constants fromconstants.mjs
improves maintainability and consistency.
30-34
: Previous suggestion on using constants foroutputFormat
options is still applicableSimilarly, consider using constants for
outputFormat
options to enhance code consistency.
83-96
: Validation to ensureheight
andwidth
are divisible by 64 is still missingThe earlier recommendation to add validation for
height
andwidth
being divisible by64
remains valid.
09e441f
to
bbb620f
Compare
/approve |
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: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
- components/runware/actions/request-task/request-task.mjs (1 hunks)
- components/runware/common/constants.mjs (1 hunks)
- components/runware/package.json (2 hunks)
- components/runware/runware.app.mjs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/runware/actions/request-task/request-task.mjs
- components/runware/common/constants.mjs
- components/runware/package.json
bbb620f
to
7f6c413
Compare
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
- components/runware/actions/request-task/request-task.mjs (1 hunks)
- components/runware/common/constants.mjs (1 hunks)
- components/runware/package.json (2 hunks)
- components/runware/runware.app.mjs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/runware/actions/request-task/request-task.mjs
- components/runware/common/constants.mjs
- components/runware/package.json
seedImage: { | ||
type: "string", | ||
label: "Seed Image", | ||
description: "When doing Image-to-Image, Inpainting or Outpainting, this parameter is **required**. Specifies the seed image to be used for the diffusion process. The image can be specified in one of the following formats:\n - An UUID v4 string of a [previously uploaded image](https://docs.runware.ai/en/getting-started/image-upload) or a [generated image](https://docs.runware.ai/en/image-inference/api-reference).\n - A data URI string representing the image. The data URI must be in the format `data:<mediaType>;base64,` followed by the base64-encoded image. For example: `data:image/png;base64,iVBORw0KGgo...`.\n - A base64 encoded image without the data URI prefix. For example: `iVBORw0KGgo...`.\n - A URL pointing to the image. The image must be accessible publicly. Supported formats are: PNG, JPG and WEBP.", | ||
optional: true, | ||
}, |
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.
Inconsistency: seedImage
is marked optional but described as required
The description states that seedImage
is required when performing Image-to-Image, Inpainting, or Outpainting tasks, but the property is marked as optional: true
. To ensure consistency and proper validation, consider adjusting the property definition to conditionally require seedImage
based on the taskType
.
maskImage: { | ||
type: "string", | ||
label: "Mask Image", | ||
description: "When doing Inpainting or Outpainting, this parameter is **required**. Specifies the mask image to be used for the inpainting process. The image can be specified in one of the following formats:\n - An UUID v4 string of a [previously uploaded image](https://docs.runware.ai/en/getting-started/image-upload) or a [generated image](https://docs.runware.ai/en/image-inference/api-reference).\n - A data URI string representing the image. The data URI must be in the format `data:<mediaType>;base64,` followed by the base64-encoded image. For example: `data:image/png;base64,iVBORw0KGgo...`.\n - A base64 encoded image without the data URI prefix. For example: `iVBORw0KGgo...`.\n - A URL pointing to the image. The image must be accessible publicly. Supported formats are: PNG, JPG and WEBP.", | ||
optional: true, | ||
}, |
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.
Inconsistency: maskImage
is marked optional but described as required
Similar to seedImage
, the maskImage
property is marked as optional: true
, but the description indicates it is required for Inpainting or Outpainting tasks. Ensure the property accurately reflects its required status based on the taskType
to prevent potential runtime errors due to missing parameters.
height: { | ||
type: "integer", | ||
label: "Height", | ||
description: "Used to define the height dimension of the generated image. Certain models perform better with specific dimensions. The value must be divisible by 64, eg: `512`, `576`, `640` ... `2048`.", |
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.
Standardize abbreviation in height
and width
descriptions
In the descriptions for the height
and width
properties, "eg" should be standardized to "e.g.," for consistency and correctness.
Apply this diff to correct the abbreviation:
// For the `height` property
- description: "Used to define the height dimension of the generated image. Certain models perform better with specific dimensions. The value must be divisible by 64, eg: `512`, `576`, `640` ... `2048`.",
+ description: "Used to define the height dimension of the generated image. Certain models perform better with specific dimensions. The value must be divisible by 64, e.g., `512`, `576`, `640` ... `2048`.",
// For the `width` property
- description: "Used to define the width dimension of the generated image. Certain models perform better with specific dimensions. The value must be divisible by 64, eg: `512`, `576`, `640` ... `2048`.",
+ description: "Used to define the width dimension of the generated image. Certain models perform better with specific dimensions. The value must be divisible by 64, e.g., `512`, `576`, `640` ... `2048`.",
Also applies to: 93-93
scheduler: { | ||
type: "string", | ||
label: "Scheduler", | ||
description: "An scheduler is a component that manages the inference process. Different schedulers can be used to achieve different results like more detailed images, faster inference, or more accurate results. The default scheduler is the one that the model was trained with, but you can choose a different one to get different results. Schedulers are explained in more detail in the [Schedulers page](https://docs.runware.ai/en/image-inference/schedulers).", |
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.
Typographical error in scheduler
description
In the description for the scheduler
property, "An scheduler" should be "A scheduler" to improve readability.
Apply this diff to correct the typo:
- description: "An scheduler is a component that manages the inference process..."
+ description: "A scheduler is a component that manages the inference process..."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
description: "An scheduler is a component that manages the inference process. Different schedulers can be used to achieve different results like more detailed images, faster inference, or more accurate results. The default scheduler is the one that the model was trained with, but you can choose a different one to get different results. Schedulers are explained in more detail in the [Schedulers page](https://docs.runware.ai/en/image-inference/schedulers).", | |
description: "A scheduler is a component that manages the inference process. Different schedulers can be used to achieve different results like more detailed images, faster inference, or more accurate results. The default scheduler is the one that the model was trained with, but you can choose a different one to get different results. Schedulers are explained in more detail in the [Schedulers page](https://docs.runware.ai/en/image-inference/schedulers).", |
WHY
Resolves #14361
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores