[Components] runpod #14353
[Components] runpod #14353
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. |
WalkthroughThis pull request introduces a set of new modules and functionalities for managing pods within the Runpod environment. It adds actions for creating, starting, stopping, and retrieving pod details via GraphQL mutations and queries. Additionally, utility functions for input processing are included, along with updates to the package version and dependencies to support the new features. Changes
Assessment against linked issues
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Outside diff range and nitpick comments (8)
components/runpod/common/mutations.mjs (2)
4-10: LGTM: createPod mutation is well-definedThe createPod mutation is correctly structured and aligns with the PR objective. It takes the appropriate input type and calls the correct resolver.
Consider if returning additional fields besides
idwould be beneficial for the client. For example, returning the pod's status or other relevant details could save an additional API call in some scenarios.
1-25: LGTM: Well-structured mutations fileThe overall structure of the file is clean, organized, and follows good practices:
- Exports a single object containing all mutations
- Each mutation is clearly defined as a property
- Adheres to the single responsibility principle
- Allows for easy addition of future mutations
Consider adding JSDoc comments for each mutation to provide more context and improve documentation. For example:
/** * Creates a new pod. * @param {PodRentInterruptableInput} input - The input parameters for creating a pod. * @returns {Promise<{id: string}>} The ID of the created pod. */ createPod: gql`...`,This would enhance the developer experience when using these mutations in other parts of the codebase.
components/runpod/actions/get-pod/get-pod.mjs (1)
4-9: LGTM: Action metadata is well-defined.The metadata for the "Get Pod Details" action is complete and follows a standard format. The inclusion of a documentation link in the description is particularly helpful.
Consider capitalizing "Successfully" in the description for consistency with other text.
components/runpod/actions/stop-pod/stop-pod.mjs (1)
34-51: LGTM with a minor typo: The run method is well-implemented.The
runmethod is correctly structured and implemented:
- It's asynchronous and destructures necessary properties.
- It calls
stopPodwith the appropriate input.- It exports a summary message and returns the response.
However, there's a minor typo in the summary message:
- $.export("$summary", `Succesfully stopped pod with ID \`${response.id}\`.`); + $.export("$summary", `Successfully stopped pod with ID \`${response.id}\`.`);Please correct the spelling of "Successfully" in the summary message.
components/runpod/common/utils.mjs (1)
53-74: ApproveparseArrayfunction with minor suggestions.The
parseArrayfunction effectively handles various input cases and provides clear error messages. It's well-implemented overall.Consider a minor refactoring for simplicity:
function parseArray(value) { if (!value) return; if (Array.isArray(value)) return value; try { const parsedValue = JSON.parse(value); if (!Array.isArray(parsedValue)) { throw new Error("Not an array"); } return parsedValue; } catch (e) { throw new ConfigurationError("Make sure the custom expression contains a valid JSON array object"); } }This refactoring maintains the original functionality while slightly reducing nesting and improving readability.
components/runpod/actions/start-pod/start-pod.mjs (2)
51-51: Confirm the necessity of parsingbidPerGpuThe
bidPerGpuproperty is being parsed usingparseFloat(bidPerGpu). IfbidPerGpuis already a number, parsing may be unnecessary. Please verify whether this parsing is required or ifbidPerGpuis ensured to be a numeric value.
55-55: Fix the misspelling in the success messageThere's a typo in the word "Successfully" in the export summary message.
Apply this diff to correct the typo:
- $.export("$summary", `Sucessfully started pod with ID \`${response.id}\`.`); + $.export("$summary", `Successfully started pod with ID \`${response.id}\`.`);components/runpod/runpod.app.mjs (1)
14-18: Clarify 'gpuCount' property type and validationThe
gpuCountproperty is set to type"integer", which is appropriate. However, consider adding validation to ensure that the value is non-negative and aligns with available GPU counts.Optionally, you can add a
minproperty for validation:gpuCount: { type: "integer", label: "GPU Count", description: "The number of GPUs to allocate to the pod. Set to 0 for pods without GPU.", + min: 0, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
- components/runpod/actions/create-pod/create-pod.mjs (1 hunks)
- components/runpod/actions/get-pod/get-pod.mjs (1 hunks)
- components/runpod/actions/start-pod/start-pod.mjs (1 hunks)
- components/runpod/actions/stop-pod/stop-pod.mjs (1 hunks)
- components/runpod/common/mutations.mjs (1 hunks)
- components/runpod/common/queries.mjs (1 hunks)
- components/runpod/common/utils.mjs (1 hunks)
- components/runpod/package.json (2 hunks)
- components/runpod/runpod.app.mjs (1 hunks)
🧰 Additional context used
🪛 Biome
components/runpod/common/utils.mjs
[error] 13-13: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (22)
components/runpod/package.json (3)
3-3: Version update looks good.The version bump from 0.0.1 to 0.1.0 is appropriate, indicating a minor release with new features. This aligns well with the addition of new pod management functionalities described in the PR objectives.
14-14: PublishConfig modification is correct.Setting "access" to "public" in publishConfig is appropriate for this open-source component, ensuring it can be published and accessed publicly on the npm registry.
15-18: New dependencies align with PR objectives.The addition of GraphQL-related dependencies is appropriate for implementing the pod management actions. However, it's important to ensure we're using the latest stable versions and that they don't have any known vulnerabilities.
Please run the following script to verify the versions and check for any known vulnerabilities:
components/runpod/common/mutations.mjs (3)
1-1: LGTM: Correct import statementThe import of
gqlfrom 'graphql-request' is appropriate for defining GraphQL mutations.
11-17: LGTM with questions: startPod mutation structureThe startPod mutation is correctly structured and aligns with the PR objective. However, there are two points that require clarification:
- The input type
PodBidResumeInputseems unusual for a start operation. Is this the correct input type, or should it be something likePodStartInput?- The resolver name
podResumedoesn't exactly match the mutation namestartPod. Is this intentional, or should they be aligned?Could you please confirm if these naming conventions are consistent with the Runpod API documentation or other parts of the codebase?
18-24: LGTM: stopPod mutation is well-definedThe stopPod mutation is correctly structured and aligns with the PR objective. The input type and resolver name are consistent with the mutation name, which is good.
components/runpod/actions/get-pod/get-pod.mjs (4)
1-2: LGTM: Imports are appropriate and well-structured.The imports for
appandqueriesare correctly sourced from their respective locations, indicating a well-organized project structure.
10-19: LGTM: Props are well-defined and use appropriate structures.The
appandpodIdprops are correctly defined. The use ofpropDefinitionforpodIdpromotes reusability and consistency across the application. The description forpodIdis clear and informative.
20-27: LGTM: The getPodDetails method is well-implemented.The
getPodDetailsmethod effectively encapsulates the API call logic, promoting code reusability and maintainability. The use ofthis.app.makeRequestwithqueries.getPoddemonstrates a consistent approach to handling API requests.
1-42: Overall, excellent implementation of the "Get Pod Details" action.This file successfully implements the "Get Pod Details" action, aligning perfectly with the PR objectives. The code is well-structured, follows best practices, and makes good use of reusable components. Only minor improvements were suggested (fixing a typo and considering capitalization in the description).
The implementation effectively addresses the "Get Pod" functionality outlined in the PR objectives, providing a clean and efficient way to retrieve pod details by ID.
components/runpod/actions/stop-pod/stop-pod.mjs (5)
1-2: LGTM: Imports are appropriate and well-structured.The imports for
appandmutationsare correctly sourced from their respective locations, indicating a well-organized project structure.
4-9: LGTM: Action metadata is well-defined and informative.The action metadata is complete, including a descriptive name, clear description with a documentation link, and appropriate versioning. This provides good context for users of the action.
10-25: LGTM: Props are well-defined and align with the action's purpose.The props are appropriately defined:
appis included for consistency with other actions.podIduses a propDefinition, promoting reusability.incrementVersionis optional, providing flexibility for users.These props align well with the action's purpose of stopping a pod.
26-33: LGTM: The stopPod method is well-implemented.The
stopPodmethod:
- Encapsulates the API call logic, promoting clean code.
- Uses the imported mutations, ensuring consistency with the rest of the system.
- Accepts variables, allowing for flexible usage.
This implementation follows good practices for API interaction.
1-52: Overall: Well-implemented "Stop Pod" action meeting PR objectives.This implementation of the "Stop Pod" action successfully fulfills the objectives outlined in the PR summary and linked issue #14304. The code is well-structured, follows good practices, and provides the necessary functionality to stop a running pod.
Key points:
- Proper action metadata and documentation link.
- Well-defined props with reusable definitions.
- Encapsulated API call logic in the
stopPodmethod.- Correctly implemented
runmethod following Pipedream action structure.The only improvement needed is fixing the minor typo in the summary message. Great job on implementing this action!
components/runpod/common/utils.mjs (2)
76-79: Approve export statement.The export statement correctly includes the necessary functions and provides an enhanced
parseArrayfunction that aligns with the PR objectives for improved pod management capabilities.The additional mapping of
parseArrayoutput throughvalueToObjectensures that any nested JSON structures within the array are properly parsed, which is a valuable feature for handling complex pod configurations.
1-79: Summary: Utility functions align well with PR objectives.The introduced utility functions in
utils.mjsprovide essential capabilities for processing input data in the Runpod component. These functions, particularlycleanInputandparseArray, directly support the PR objectives of enhancing pod management capabilities by ensuring robust handling of user inputs and configuration data.While there are opportunities for minor improvements in naming and implementation details, the overall functionality aligns well with the requirements for creating, starting, stopping, and retrieving pod details as outlined in the PR objectives.
To fully meet the PR objectives, ensure that these utility functions are properly integrated into the main Runpod component actions (create, start, stop, and get pod) to handle user inputs and API responses effectively.
🧰 Tools
🪛 Biome
[error] 13-13: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
components/runpod/common/queries.mjs (3)
76-85: LGTM! Efficient query for listing pods.The
listPodsquery is well-designed for its purpose. It efficiently retrieves only the necessary information (id and name) for listing pods, which aligns with the PR objective of managing pods.
86-93: LGTM! Please clarify the addition of GPU types listing.The
listGpuTypesquery is well-structured and provides necessary information for GPU type selection. This aligns with the overall goal of pod management.However, the addition of this query wasn't explicitly mentioned in the PR objectives or linked issues. Could you please clarify the reason for including this functionality?
To verify the necessity of this query, let's check if GPU types are used elsewhere in the component:
4-75: LGTM! Consider security implications of sensitive data retrieval.The
getPodquery is well-structured and comprehensive, aligning with the PR objective of managing pods. It covers a wide range of pod attributes and uses fragments for better code organization.However, please verify the security implications of retrieving sensitive information like
apiKeyin the query. Consider implementing additional access controls or data masking for such fields.Run the following script to check for other instances of sensitive data retrieval:
components/runpod/actions/start-pod/start-pod.mjs (1)
7-7: Verify the correctness of the documentation linkThe documentation link in the description refers to "start-spot-pod". Please confirm that this is the correct link for starting a pod. If the action applies to all pods, consider updating the link to the appropriate section.
components/runpod/actions/create-pod/create-pod.mjs (1)
179-179:⚠️ Potential issueEnsure correct access to 'id' in the response
The
responseobject returned bycreatePodmay not have theidproperty directly at the top level. Depending on the structure of the response, you might need to accessresponse.data.createPod.id. Please verify the response structure and adjust the code accordingly to avoid runtime errors.Apply this diff if needed:
- $.export("$summary", `Successfully created a new pod with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created a new pod with ID \`${response.data.createPod.id}\`.`);Verification Script:
| async run({ $ }) { | ||
| const { | ||
| getPodDetails, | ||
| podId, | ||
| } = this; | ||
|
|
||
| const response = await getPodDetails({ | ||
| input: { | ||
| podId, | ||
| }, | ||
| }); | ||
| $.export("$summary", `Succesfully retrieved details for pod with ID \`${response.id}\`.`); | ||
| return response; | ||
| }, |
There was a problem hiding this comment.
LGTM: The run function is well-structured and follows best practices.
The async function is implemented correctly, using destructuring for improved readability. The API call and summary export provide the necessary functionality and user feedback.
There's a typo in the summary message. Please apply the following fix:
- $.export("$summary", `Succesfully retrieved details for pod with ID \`${response.id}\`.`);
+ $.export("$summary", `Successfully retrieved details for pod with ID \`${response.id}\`.`);📝 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.
| async run({ $ }) { | |
| const { | |
| getPodDetails, | |
| podId, | |
| } = this; | |
| const response = await getPodDetails({ | |
| input: { | |
| podId, | |
| }, | |
| }); | |
| $.export("$summary", `Succesfully retrieved details for pod with ID \`${response.id}\`.`); | |
| return response; | |
| }, | |
| async run({ $ }) { | |
| const { | |
| getPodDetails, | |
| podId, | |
| } = this; | |
| const response = await getPodDetails({ | |
| input: { | |
| podId, | |
| }, | |
| }); | |
| $.export("$summary", `Successfully retrieved details for pod with ID \`${response.id}\`.`); | |
| return response; | |
| }, |
| function cleanInput(input) { | ||
| return Object.fromEntries( | ||
| Object.entries(input) | ||
| .filter(([ | ||
| , value, | ||
| ]) => value !== null && value !== undefined) | ||
| .map(([ | ||
| key, | ||
| value, | ||
| ]) => { | ||
| if (typeof value === "string" && !isNaN(value)) { | ||
| const parsedValue = | ||
| value.includes(".") | ||
| ? parseFloat(value) | ||
| : parseInt(value, 10); | ||
| return [ | ||
| key, | ||
| parsedValue, | ||
| ]; | ||
| } | ||
| return [ | ||
| key, | ||
| value, | ||
| ]; | ||
| }), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor cleanInput function for improved safety and readability.
The cleanInput function effectively filters and transforms input data. However, there are a few improvements we can make:
- Replace
isNaNwithNumber.isNaNfor safer number checking. - Use optional chaining and nullish coalescing for a more concise implementation.
- Consider using
Object.entries(input).reduce()for a more straightforward approach.
Here's a suggested refactoring:
function cleanInput(input) {
return Object.entries(input).reduce((acc, [key, value]) => {
if (value != null) {
if (typeof value === "string" && !Number.isNaN(Number(value))) {
acc[key] = value.includes(".") ? parseFloat(value) : parseInt(value, 10);
} else {
acc[key] = value;
}
}
return acc;
}, {});
}This refactoring addresses the isNaN issue, simplifies the logic, and maintains the original functionality.
🧰 Tools
🪛 Biome
[error] 13-13: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
| function isJson(value) { | ||
| value = | ||
| typeof(value) !== "string" | ||
| ? JSON.stringify(value) | ||
| : value; | ||
|
|
||
| try { | ||
| value = JSON.parse(value); | ||
| } catch (e) { | ||
| return false; | ||
| } | ||
|
|
||
| return typeof(value) === "object" && value !== null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve isJson function for clarity and correctness.
The isJson function correctly identifies JSON objects, but there are a few points to consider:
- The function name
isJsonis slightly misleading as it specifically checks for JSON objects, not all valid JSON (which could include primitives). - Modifying the input
valuecould lead to unexpected behavior if the function is used in a context where the original value needs to be preserved.
Consider renaming the function to isJsonObject and refactoring it as follows:
function isJsonObject(value) {
if (typeof value !== "string") {
return false;
}
try {
const result = JSON.parse(value);
return typeof result === "object" && result !== null;
} catch (e) {
return false;
}
}This refactoring preserves the original input, clarifies the function's purpose, and maintains the core functionality.
| function valueToObject(value) { | ||
| if (!isJson(value)) { | ||
| return value; | ||
| } | ||
| return JSON.parse(value); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refine valueToObject function for clarity.
The valueToObject function correctly handles the conversion of JSON strings to objects. However, we can make a few improvements:
- Rename the function to
parseJsonObjectto better reflect its functionality. - Update the function to use the renamed
isJsonObjectfunction (as suggested in the previous comment).
Here's a suggested refactoring:
function parseJsonObject(value) {
return isJsonObject(value) ? JSON.parse(value) : value;
}This refactoring maintains the original functionality while improving clarity and consistency with the other utility functions.
| const response = await startPod({ | ||
| input: { | ||
| podId, | ||
| gpuCount, | ||
| bidPerGpu: parseFloat(bidPerGpu), | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for the API request
The startPod API call may fail, and currently, potential errors are not being handled. Consider adding error handling to manage exceptions and provide meaningful feedback to the user.
You could wrap the API call in a try-catch block:
async run({ $ }) {
const {
startPod,
podId,
gpuCount,
bidPerGpu,
} = this;
+ try {
const response = await startPod({
input: {
podId,
gpuCount,
bidPerGpu: parseFloat(bidPerGpu),
},
});
$.export("$summary", `Successfully started pod with ID \`${response.id}\`.`);
return response;
+ } catch (error) {
+ $.export("$summary", "Failed to start pod.");
+ throw new Error(`Error starting pod: ${error.message}`);
+ }
},Committable suggestion was skipped due to low confidence.
| async options() { | ||
| const { myself: { pods } } = await this.listPods(); | ||
| return pods.map(({ | ||
| id: value, name: label, | ||
| }) => ({ | ||
| label: label || value, | ||
| value, | ||
| })); | ||
| }, |
There was a problem hiding this comment.
Ensure proper error handling in 'podId' options method
The options method for podId may throw an error if listPods fails or returns an unexpected response. Adding error handling will improve the reliability of this method.
Consider adding a try-catch block:
async options() {
- const { myself: { pods } } = await this.listPods();
+ try {
+ const { myself: { pods } } = await this.listPods();
+ return pods.map(({
+ id: value, name: label,
+ }) => ({
+ label: label || value,
+ value,
+ }));
+ } catch (error) {
+ console.error("Failed to retrieve pods:", error);
+ return [];
+ }
},📝 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.
| async options() { | |
| const { myself: { pods } } = await this.listPods(); | |
| return pods.map(({ | |
| id: value, name: label, | |
| }) => ({ | |
| label: label || value, | |
| value, | |
| })); | |
| }, | |
| async options() { | |
| try { | |
| const { myself: { pods } } = await this.listPods(); | |
| return pods.map(({ | |
| id: value, name: label, | |
| }) => ({ | |
| label: label || value, | |
| value, | |
| })); | |
| } catch (error) { | |
| console.error("Failed to retrieve pods:", error); | |
| return []; | |
| } | |
| }, |
| ports: { | ||
| type: "string", | ||
| label: "Ports", | ||
| description: "The ports to use for the pod.", | ||
| }, |
There was a problem hiding this comment.
Clarify the expected format for 'Ports' property
The Ports property is defined as a string, but it's unclear how users should input the port values. Should it be a single port number, a comma-separated list, or another format? Please provide clear guidance in the description to help users enter the correct values.
Apply this diff to update the description:
- description: "The ports to use for the pod.",
+ description: "The ports to use for the pod. Provide a comma-separated list of port numbers (e.g., '80,443,8080').",📝 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.
| ports: { | |
| type: "string", | |
| label: "Ports", | |
| description: "The ports to use for the pod.", | |
| }, | |
| ports: { | |
| type: "string", | |
| label: "Ports", | |
| description: "The ports to use for the pod. Provide a comma-separated list of port numbers (e.g., '80,443,8080').", | |
| }, |
| env: { | ||
| type: "string[]", | ||
| label: "Environment Variables", | ||
| description: "The environment variables to set for the pod. Each row should be formated with JSON string like this `{\"key\": \"ENV_NAME\", \"value\": \"ENV_VALUE\"}`", |
There was a problem hiding this comment.
Typo in description: 'formated' should be 'formatted'
There's a typographical error in the description of the Environment Variables property. Please correct 'formated' to 'formatted'.
Apply this diff to fix the typo:
- description: "The environment variables to set for the pod. Each row should be formated with JSON string like this `{\"key\": \"ENV_NAME\", \"value\": \"ENV_VALUE\"}`",
+ description: "The environment variables to set for the pod. Each row should be formatted with a JSON string like this `{\"key\": \"ENV_NAME\", \"value\": \"ENV_VALUE\"}`",📝 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: "The environment variables to set for the pod. Each row should be formated with JSON string like this `{\"key\": \"ENV_NAME\", \"value\": \"ENV_VALUE\"}`", | |
| description: "The environment variables to set for the pod. Each row should be formatted with a JSON string like this `{\"key\": \"ENV_NAME\", \"value\": \"ENV_VALUE\"}`", |
| type: "string", | ||
| label: "Stop After", | ||
| description: "The duration after which the pod will be stopped.", | ||
| optional: true, |
There was a problem hiding this comment.
Clarify the expected format for 'Stop After' property
The Stop After property is of type string and represents the duration after which the pod will be stopped. Please specify the expected format for this duration (e.g., seconds, ISO 8601 duration) to help users provide the correct input.
Apply this diff to improve the description:
- description: "The duration after which the pod will be stopped.",
+ description: "The duration in seconds after which the pod will be stopped (e.g., '3600' for 1 hour).",📝 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.
| type: "string", | |
| label: "Stop After", | |
| description: "The duration after which the pod will be stopped.", | |
| optional: true, | |
| type: "string", | |
| label: "Stop After", | |
| description: "The duration in seconds after which the pod will be stopped (e.g., '3600' for 1 hour).", | |
| optional: true, |
| env: { | ||
| type: "string[]", | ||
| label: "Environment Variables", | ||
| description: "The environment variables to set for the pod. Each row should be formated with JSON string like this `{\"key\": \"ENV_NAME\", \"value\": \"ENV_VALUE\"}`", | ||
| optional: true, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve handling of 'Environment Variables' property to avoid parsing JSON strings
Currently, the Environment Variables property expects users to input JSON-formatted strings, which can be error-prone. Consider changing the property to accept key-value pairs directly, improving usability and reducing the likelihood of input errors.
Apply this refactor to change the Environment Variables property:
- type: "string[]",
- label: "Environment Variables",
- description: "The environment variables to set for the pod. Each row should be formated with JSON string like this `{\"key\": \"ENV_NAME\", \"value\": \"ENV_VALUE\"}`",
- optional: true,
+ type: "object[]",
+ label: "Environment Variables",
+ description: "The environment variables to set for the pod. Each item should be an object with 'key' and 'value' properties.",
+ optional: true,Then, in your run method, you can use env directly without parsing:
- env: utils.parseArray(env),
+ env,Committable suggestion was skipped due to low confidence.
1 participant
WHY
Resolves #14304
Summary by CodeRabbit
New Features
Version Update