-
Couldn't load subscription status.
- Fork 5.5k
New Components - hamsa #16226
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
New Components - hamsa #16226
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces several new modules and updates to the Hamsa component. New action modules for creating AI content, transcribing videos, listing jobs, and synthesizing voice have been added. A constants file has been introduced to centralize configuration options, and the main Hamsa application has been updated with new properties and methods to standardize API interactions. Additionally, the package version has been bumped and a new dependency has been added. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/Client
participant A as Action Module
participant H as Hamsa App
participant API as External API
U->>A: Trigger Action (e.g., create-content)
A->>H: Invoke method (e.g., createAIContent)
H->>API: Execute _makeRequest with endpoint and parameters
API-->>H: Return API response
H-->>A: Forward response data
A-->>U: Return summary message
Assessment against linked issues
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/hamsa/actions/synthesize-voice/synthesize-voice.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/facebook_graph_api/facebook_graph_api.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/gather/gather.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ Finishing Touches
🪧 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 (
|
Actions - Transcribe Video - Create Content - Synthethize Voice
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: 7
🧹 Nitpick comments (6)
components/hamsa/common/constants.mjs (1)
14-17: Consider consistent structure for LANGUAGE_OPTIONS.Unlike MODEL_OPTIONS and AI_PARTS_OPTIONS which use objects with label/value pairs, LANGUAGE_OPTIONS uses simple strings. Consider using the same object structure for consistency.
-export const LANGUAGE_OPTIONS = [ - "ar", - "en", -]; +export const LANGUAGE_OPTIONS = [ + { + label: "Arabic", + value: "ar", + }, + { + label: "English", + value: "en", + }, +];components/hamsa/actions/synthethize-voice/synthethize-voice.mjs (1)
42-61: Optimize webhook authentication handling.The current implementation can be improved by:
- Only adding webhookUrl to data if it's provided
- Simplifying the webhookAuth logic
async run({ $ }) { - const webhookAuth = {}; - if (this.webhookAuthKey) { - webhookAuth.authKey = this.webhookAuthKey; - } - if (this.webhookAuthSecret) { - webhookAuth.authSecret = this.webhookAuthSecret; - } const data = { voiceId: this.voiceId, text: this.text, - webhookUrl: this.webhookUrl, }; - if (Object.keys(webhookAuth).length) { - data.webhookAuth = webhookAuth; + + if (this.webhookUrl) { + data.webhookUrl = this.webhookUrl; + } + + if (this.webhookAuthKey || this.webhookAuthSecret) { + data.webhookAuth = {}; + if (this.webhookAuthKey) { + data.webhookAuth.authKey = this.webhookAuthKey; + } + if (this.webhookAuthSecret) { + data.webhookAuth.authSecret = this.webhookAuthSecret; + } } const response = await this.hamsa.generateTTS({ $, data, });components/hamsa/actions/transcribe-video/transcribe-video.mjs (2)
7-12: Component metadata looks good, but consider incrementing the version from 0.0.1The component is well-defined with a descriptive name, documentation link, and proper metadata. However, consider starting with version "0.0.2" instead of "0.0.1" to allow for potential bug fixes without breaking semantic versioning principles.
68-78: Consider not including undefined values in the request dataThe current implementation adds all props to the data object even if they're undefined/null, which could lead to API errors if the backend doesn't handle null values properly.
- const data = { - mediaUrl: this.mediaUrl, - model: this.model, - processingType: "async", - webhookUrl: this.webhookUrl, - title: this.title, - language: this.language, - }; + const data = { + mediaUrl: this.mediaUrl, + model: this.model, + processingType: "async", + }; + + if (this.webhookUrl) data.webhookUrl = this.webhookUrl; + if (this.title) data.title = this.title; + if (this.language) data.language = this.language; + if (Object.keys(webhookAuth).length) { data.webhookAuth = webhookAuth; }components/hamsa/hamsa.app.mjs (2)
58-65: Consider adding API version to baseUrl as a configurable constantThe API URL and version are hardcoded. Consider extracting the version to a constant to make future API version upgrades easier to manage.
+ _apiVersion() { + return "v1"; + }, _baseUrl() { - return "https://api.tryhamsa.com/v1"; + return `https://api.tryhamsa.com/${this._apiVersion()}`; },
75-92: Add caching for project ID to reduce API callsThe
listJobsmethod makes an API call to get the project ID for every request. Consider caching this value to improve performance.+ async _getProjectId(opts = {}) { + if (!this._projectId) { + const { data: { id } } = await this._makeRequest({ + path: "/projects/by-api-key", + ...opts, + }); + this._projectId = id; + } + return this._projectId; + }, async listJobs({ params, ...opts }) { - const { data: { id: projectId } } = await this._makeRequest({ - path: "/projects/by-api-key", - ...opts, - }); + const projectId = await this._getProjectId(opts); return this._makeRequest({ method: "POST", path: "/jobs/all",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
components/hamsa/actions/create-content/create-content.mjs(1 hunks)components/hamsa/actions/synthethize-voice/synthethize-voice.mjs(1 hunks)components/hamsa/actions/transcribe-video/transcribe-video.mjs(1 hunks)components/hamsa/common/constants.mjs(1 hunks)components/hamsa/hamsa.app.mjs(1 hunks)components/hamsa/package.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (9)
components/hamsa/package.json (2)
3-3: Version increase shows proper semantic versioning.The version bump from 0.0.1 to 0.1.0 correctly follows semantic versioning principles, indicating the addition of new features without breaking changes.
14-16: Dependency addition is appropriate.Adding the @pipedream/platform dependency is necessary for the new components being introduced. The caret (^) notation allows for compatible updates, which is a good practice.
components/hamsa/common/constants.mjs (3)
1-1: LIMIT constant is well-defined.The export of a constant with a specific limit value is a good practice for maintaining consistent pagination or other limit values throughout the application.
3-12: MODEL_OPTIONS follows good structure with descriptive labels.The model options include detailed descriptive labels that explain the purpose and optimization targets of each ASR model, which is very helpful for users.
19-68: Comprehensive set of AI content part options.The AI_PARTS_OPTIONS provides a well-structured and comprehensive set of content generation options covering various content types and platforms, which gives users flexibility in content creation.
components/hamsa/actions/create-content/create-content.mjs (1)
1-2: Imports look good.Properly importing the constants from the common directory and the Hamsa app module.
components/hamsa/actions/transcribe-video/transcribe-video.mjs (1)
13-59: Props are well-defined with clear descriptionsThe props are well-structured with appropriate types, labels and descriptions. Good job on making the webhook authentication fields optional and marking the auth secret with
secret: truefor security.components/hamsa/hamsa.app.mjs (2)
7-56: Property definitions and async options are well implementedThe property definitions for jobId, voiceId, and webhookUrl are well structured with clear labels and descriptions. The async options implementation for retrieving job IDs and voice IDs with pagination support is particularly good.
66-74: Good implementation of the _makeRequest helper methodThe _makeRequest method is a good abstraction that centralizes request logic and reduces code duplication. It appropriately handles headers and base URL construction.
components/hamsa/actions/synthethize-voice/synthethize-voice.mjs
Outdated
Show resolved
Hide resolved
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.
It would be nice to have an action for getting the job results (https://docs.tryhamsa.com/api-reference/endpoint/get-job-by-id). Do you think that would be helpful?
What about possibly adding a callbackWithRerun prop to optionally use Pipedream's $.flow.rerun to wait for the job results? Example
components/hamsa/actions/synthethize-voice/synthethize-voice.mjs
Outdated
Show resolved
Hide resolved
Hi @michelle0927, I'm not sure if using $.flow is a good idea, as some videos can be very long and it can take a long time to get a response. |
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: 1
🧹 Nitpick comments (3)
components/hamsa/actions/list-jobs/list-jobs.mjs (3)
12-20: Consider adding error handling.The current implementation does not include explicit error handling. Adding try/catch blocks would provide better user feedback when API errors occur.
async run({ $ }) { + try { const response = await this.hamsa.listJobs({ $, }); $.export("$summary", `Successfully fetched ${response?.data?.jobs?.length} jobs.`); return response; + } catch (error) { + $.export("$summary", `Failed to fetch jobs: ${error.message}`); + throw error; + } },
1-21: Consider enhancing the component with pagination support.Based on the PR objectives mentioning API integration, if the Hamsa API supports pagination for listing jobs, you might want to add pagination parameters to improve usability for users with many jobs.
You could add optional props like
limitandpageoroffsetthat would be passed to thelistJobsmethod:props: { hamsa, + limit: { + type: "integer", + label: "Limit", + description: "Maximum number of jobs to return", + optional: true, + }, + page: { + type: "integer", + label: "Page", + description: "Page number for paginated results", + optional: true, + }, }, async run({ $ }) { const response = await this.hamsa.listJobs({ $, + limit: this.limit, + page: this.page, });
1-21: Consider implementing filtering options.To make this component more useful, you might want to add filtering capabilities like filtering jobs by status, date range, or type - if supported by the Hamsa API.
For example:
props: { hamsa, + status: { + type: "string", + label: "Status", + description: "Filter jobs by status", + optional: true, + options: ["pending", "processing", "completed", "failed"], + }, + dateFrom: { + type: "string", + label: "Date From", + description: "Filter jobs created after this date (ISO format)", + optional: true, + }, }, async run({ $ }) { const response = await this.hamsa.listJobs({ $, + status: this.status, + dateFrom: this.dateFrom, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/hamsa/actions/list-jobs/list-jobs.mjs(1 hunks)components/hamsa/actions/synthethize-voice/synthethize-voice.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/hamsa/actions/synthethize-voice/synthethize-voice.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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
🧹 Nitpick comments (2)
components/hamsa/actions/synthesize-voice/synthesize-voice.mjs (2)
63-64: Add null check for response.id.The code assumes that
response.idwill always exist. Adding a null check would make the code more robust against unexpected API responses.- $.export("$summary", `Text to speech job successfully created with ID ${response.id}`); + $.export("$summary", `Text to speech job successfully created${response.id ? ` with ID ${response.id}` : ''}`);
22-40: Ensure webhook authentication is properly documented.The component includes fields for webhook authentication, but it would be helpful to document how these fields are used by the Hamsa API and any specific format requirements.
Consider updating the descriptions to provide more specific guidance on the format and requirements of the webhook authentication fields, such as:
- description: "Authorization key for the webhook notifications.", + description: "Authorization key for the webhook notifications. This will be included in the webhook request headers for authentication.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/hamsa/actions/synthesize-voice/synthesize-voice.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (1)
components/hamsa/actions/synthesize-voice/synthesize-voice.mjs (1)
3-8: Consider implementing the callbackWithRerun functionality.Based on the PR objectives, there was a suggestion to implement a
callbackWithRerunproperty to utilize Pipedream's$.flow.rerunfunctionality for waiting for job results, similar to the AssemblyAI implementation. This would be valuable for users who need to wait for job completion.Would implementing a rerun mechanism be valuable for this component to check job status until completion? If so, consider adding a
callbackWithRerunproperty and implementation logic similar to what was mentioned in the PR comments.
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: 0
♻️ Duplicate comments (1)
components/hamsa/hamsa.app.mjs (1)
93-119: API method implementations lack proper error handling.The API methods (listVoices, transcribeVideo, generateTTS, createAIContent) are consistently implemented, but they don't include any error handling. Consider adding try/catch blocks or documenting that errors should be handled by the calling code.
For better error handling, consider updating one or more of these methods:
transcribeVideo(opts = {}) { - return this._makeRequest({ - method: "POST", - path: "/jobs/transcribe", - ...opts, - }); + try { + return this._makeRequest({ + method: "POST", + path: "/jobs/transcribe", + ...opts, + }); + } catch (error) { + const errorMessage = error.response?.data?.message || error.message; + const enhancedError = new Error(`Failed to transcribe video: ${errorMessage}`); + enhancedError.originalError = error; + throw enhancedError; + } },
🧹 Nitpick comments (2)
components/hamsa/hamsa.app.mjs (2)
58-60: Consider moving the base URL to constants file.For better maintainability, consider moving the hardcoded API URL to the constants.mjs file. This would centralize all configuration values in one place.
// In common/constants.mjs + export const BASE_URL = "https://api.tryhamsa.com/v1"; // In hamsa.app.mjs _baseUrl() { - return "https://api.tryhamsa.com/v1"; + return BASE_URL; },
112-119: Consider adding a method to retrieve job results.Based on the PR objectives, there was a suggestion to add functionality for retrieving job results by ID. This would complement the existing methods and allow users to check the status of their jobs.
Would you like me to suggest an implementation for a
getJobResultsmethod to retrieve results for a specific job ID?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
components/facebook_graph_api/facebook_graph_api.app.mjs(1 hunks)components/gather/gather.app.mjs(1 hunks)components/hamsa/actions/synthesize-voice/synthesize-voice.mjs(1 hunks)components/hamsa/hamsa.app.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- components/facebook_graph_api/facebook_graph_api.app.mjs
- components/gather/gather.app.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- components/hamsa/actions/synthesize-voice/synthesize-voice.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (4)
components/hamsa/hamsa.app.mjs (4)
1-2: Good use of imports and constants.Clean import structure, with axios from the platform and LIMIT from a constants file - this promotes maintainability.
7-56: Well-structured prop definitions with good documentation.The prop definitions are well-implemented with:
- Clear type definitions
- Descriptive labels and helpful descriptions
- Proper async options implementation for dynamic dropdowns
- Good pagination handling for both jobId and voiceId
The structure for displaying job titles and voice names with tags is user-friendly.
58-74: Good API abstraction with _baseUrl, _headers and _makeRequest.These utility methods create a clean abstraction for API requests. The _makeRequest method centralizes request logic which promotes consistency.
75-92: The listJobs method properly handles the two-step process.Good implementation of first fetching the projectId and then using it in the jobs request. The pagination parameters are correctly passed through.
|
/approve |
Resolves #16189.
Summary by CodeRabbit
New Features
Chores