Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Adds portkey plugin #10337

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

feat: Adds portkey plugin #10337

wants to merge 3 commits into from
3,114 −1,073

Conversation

sarathsomana
Copy link

  • Adds plugin for Portkey APIs

@CLAassistant
Copy link

CLAassistant commented Jul 12, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@TooljetBot
Copy link
Collaborator

TooljetBot commented Jul 12, 2024 •
edited
Loading

Code Review Agent Run #8e62e1

  • AI Based Review: ✔️ Successful

Code Review Overview

  • Summary: This PR introduces several new files and updates to the Portkey plugin. A .gitignore file is added to exclude certain directories. A basic test structure is set up for the portkey module. An SVG icon file is added. The main Portkey class is implemented with methods for running queries and testing connections. Functions for handling various Portkey operations are added, and TypeScript types and interfaces are defined for these operations.
  • Files: 6
  • Issue found: Total - 8, High importance - 8      See detailed feedback ->
  • Code change type: Security, Functionality, Test Stability, Accessibility
  • Unit tests added: False
  • Estimated effort to review (1-5, lower is better): 2

High-level Feedback

Ensure that sensitive information is not exposed through console logs by using a proper logging mechanism. Validate and sanitize inputs to prevent runtime errors and security vulnerabilities. Improve accessibility by adding titles and descriptions to SVG files. Mock dependencies in tests to ensure they are isolated and reliable.

Detailed Feedback

📄 marketplace/plugins/portkey/lib/types.ts
Issues: Total - 1, High importance - 1
Line 1-62 🔴 High importance - 1   
📄 marketplace/plugins/portkey/.gitignore
Issues: Total - 1, High importance - 1
Line 1-5 🔴 High importance - 1   
📄 marketplace/plugins/portkey/lib/portkey_operations.ts
Issues: Total - 2, High importance - 2
Line 17-17 🔴 High importance - 1   
Line 33-33 🔴 High importance - 1   
📄 marketplace/plugins/portkey/lib/icon.svg
Issues: Total - 1, High importance - 1
Line 1-9 🔴 High importance - 1   
📄 marketplace/plugins/portkey/lib/index.ts
Issues: Total - 2, High importance - 2
Line 15-15 🔴 High importance - 1   
Line 32-32 🔴 High importance - 1   
📄 marketplace/plugins/portkey/__tests__/index.js
Issues: Total - 1, High importance - 1
Line 3-3 🔴 High importance - 1   

AI Code Review powered by Bito Logo

TooljetBot
TooljetBot reviewed Jul 12, 2024
View reviewed changes
marketplace/plugins/portkey/lib/types.ts Outdated
Comment on lines 1 to 62
export type SourceOptions = {
apiKey: string;
virtualKey: string;
config: Record<string, any>;
};

// export type QueryOptions = {
// operation: Operation;
// prompt?: string;
// max_tokens?: number | string;
// temperature?: number | string;
// stop_sequence?: [string];
// suffix?: string | null;
// };
interface CredentialsBase {
config?: Record<string, any> | null;
virtualKey?: string | null;
}
export enum Operation {
Completion = 'completion',
Chat = 'chat',
PromptCompletion = 'prompt_completion',
CreateEmbedding = 'create_embedding',
}
interface CompletionQueryBase extends CredentialsBase {
operation: Operation;
model: string;
temperature?: number | string;
max_tokens?: number | string;
stop_sequences?: string;
metadata?: Record<string, any> | null;
other_parameters?: Record<string, any> | null;
}

interface Message {
role: string;
content: string;
}
export interface ChatCompletionQueryOptions extends CompletionQueryBase {
messages: Array<Message>
}

export interface TextCompletionQueryOptions extends CompletionQueryBase {
prompt: string
}

export interface PromptCompletionQueryOptions extends CredentialsBase {
operation: Operation;
promptId: string;
variables?: Record<string, any>;
parameters?: Record<string, any>;
metadata?: Record<string, any>;
}

export interface EmbeddingQueryOptions extends CredentialsBase {
operation: Operation;
input: string;
model: string;
metadata?: Record<string, any>;
}

export type QueryOptions = TextCompletionQueryOptions | ChatCompletionQueryOptions | PromptCompletionQueryOptions | EmbeddingQueryOptions;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bito Code Review Agent Run #8e62e1 - 07/12/2024, 12:45 am

🔴 High importance
Issue: The 'stop_sequences' field in the 'CompletionQueryBase' interface is defined as a 'string'. This should be an array of strings to allow for multiple stop sequences, which is a common requirement in text generation APIs.
Fix: Change the type of 'stop_sequences' to 'string[]' to allow for multiple stop sequences.
Code suggestion 
 @@ -30,7  30,7 @@
  interface CompletionQueryBase extends CredentialsBase {
    operation: Operation;
    model: string;
    temperature?: number | string;
    max_tokens?: number | string;
 -  stop_sequences?: string;
    stop_sequences?: string[];
    metadata?: Record<string, any> | null;
    other_parameters?: Record<string, any> | null;
  }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if string[] is parsed out of the box from text input of Tooljet, Hence using a string to parse manually.

@github-actions GitHub Actions
Copy link

Deployment: https://tooljet-pr-10337.onrender.com
Dashboard: https://dashboard.render.com/web/srv-cqb1tdo8fa8c73av8810

@sarathsomana
Copy link
Author

@mansukh-tj Please find a short demo of operations in the plugin here :

https://www.loom.com/share/25f9db309b7141178f4437f150a1180e?sid=d24587e3-822a-4eff-bec9-d0e9887cac36
https://www.loom.com/share/c57d7933544040109896520a8550205c?sid=3784c657-d6bb-42b4-89ce-c3192a3e221f

Do let me know if you require additional information to review this PR

@akshaysasidrn
Copy link
Collaborator

/review

@TooljetBot
Copy link
Collaborator

TooljetBot commented Jul 26, 2024 •
edited
Loading

Code Review Agent Run #9e2d75

  • AI Based Review: ✔️ Successful

High-level Feedback

Ensure type safety by using generic type parameters and correct data types for fields. Validate and sanitize inputs to prevent potential issues. Consider adding unit tests to cover new functionalities and changes to ensure robustness and reliability.

AI Code Review powered by Bito Logo

@sarathsomana
Copy link
Author

/review

@akshaysasidrn Do let me know your suggestions on the PR.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 1, 2024

Deployment: https://tooljet-pr-10337.onrender.com
Dashboard: https://dashboard.render.com/web/null

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 1, 2024

Deployment: https://tooljet-pr-10337.onrender.com
Dashboard: https://dashboard.render.com/web/null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TooljetBot TooljetBot Awaiting requested review from TooljetBot

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
deploy-marketplace-plugin plugins roadmap suspend-review-app
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@sarathsomana @CLAassistant @TooljetBot @akshaysasidrn @adishM98 @mansukh-tj @SugamSrivastava