[Components] azure_storage - new components #15561
Conversation
jcortes
added
action
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces a set of new modules for Azure Storage operations, including actions for creating containers, uploading blobs, and deleting blobs. Event sources for new container creation, new blob creation, and blob deletion have also been added. The core Azure Storage application module has been enhanced with XML parsing and robust HTTP request handling, alongside new constants and utility functions. Additionally, the package configuration has been updated to reflect the new version and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant R as Runner
participant A as Action Module
participant App as AzureApp
participant API as Azure Storage API
R->>A: call run()
A->>App: invoke createContainer / deleteBlob / uploadBlob
App->>API: send HTTP request (PUT/DELETE)
API-->>App: return response
App-->>A: return success
A-->>R: return confirmation
sequenceDiagram
participant P as Poller
participant App as AzureApp
participant API as Azure Storage API
participant E as Event Emitter
P->>App: call listContainers / listBlobs
App->>API: send GET request
API-->>App: return resource list
App-->>P: provide resources
P->>E: emit event with metadata
Suggested reviewers
Poem
✨ Finishing Touches
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: 14
🧹 Nitpick comments (8)
components/azure_storage/azure_storage.app.mjs (1)
19-28: Guard against undefined containers.
Ifcontainersisundefinedor empty, this destructuring may fail or the fallback logic could produce unexpected results. Consider adding a check before mapping.const { EnumerationResults: { Containers: { Container: containers } } } = await this.listContainers(); + if (!containers) { + return []; + } return Array.isArray(containers) ? containers.map(({ Name: value }) => value) : [containers.Name];components/azure_storage/common/utils.mjs (1)
22-25: Add edge case handling togetFilenameFromPath.The function should handle edge cases like empty paths or paths without filenames.
function getFilenameFromPath(filePath) { + if (!filePath) { + throw new Error("File path cannot be empty"); + } const pathParts = filePath.split("/"); - return pathParts[pathParts.length - 1]; + const filename = pathParts[pathParts.length - 1]; + if (!filename) { + throw new Error("Invalid file path: no filename found"); + } + return filename; }components/azure_storage/sources/blob-deleted/test-event.mjs (2)
5-7: Use current or past dates in test data.The test event uses future dates (2025) which could cause confusion. Consider using current or past dates to make the test data more realistic.
- "Creation-Time": "Tue, 11 Feb 2025 23:05:34 GMT", - "Last-Modified": "Tue, 11 Feb 2025 23:05:34 GMT", + "Creation-Time": "Tue, 11 Feb 2024 23:05:34 GMT", + "Last-Modified": "Tue, 11 Feb 2024 23:05:34 GMT",
1-24: Add JSDoc documentation for the test event object.Consider adding JSDoc documentation to describe the purpose and structure of the test event.
+/** + * Sample event for a deleted blob in Azure Storage. + * This object represents the metadata and properties of a deleted blob, + * used for testing the blob-deleted event source. + * @type {Object} + */ export default {components/azure_storage/sources/new-container-created/new-container-created.mjs (1)
32-38: Add timestamp validation ingenerateMeta.The function should validate and normalize the timestamp.
generateMeta(resource) { + if (!resource?.Name) { + throw new Error("Invalid resource: missing Name"); + } + const timestamp = Date.now(); + if (isNaN(timestamp)) { + throw new Error("Invalid timestamp generated"); + } return { id: resource.Name, summary: `New Container: ${resource.Name}`, - ts: Date.now(), + ts: timestamp, }; },components/azure_storage/sources/new-blob-created/new-blob-created.mjs (1)
42-48: Use ISO timestamp for better precision and timezone support.Using
Date.now()for the timestamp loses timezone information and may cause issues with event ordering.Consider using ISO format:
generateMeta(resource) { return { id: resource.Name, summary: `New Blob: ${resource.Name}`, - ts: Date.now(), + ts: new Date().toISOString(), }; },components/azure_storage/actions/upload-blob/upload-blob.mjs (2)
12-17: Consider improving the info alert formatting.The alert content could be more readable with proper markdown formatting.
- content: "In order to have the right permissions to use this feature you need to go to the Azure Console in `Storage Account > IAM > Add role assignment`, and add the special permissions for this type of request:\n - Storage Blob Data Contributor\n - Storage Queue Data Contributor\n [See the documentation](https://learn.microsoft.com/en-us/rest/api/storageservices/put-blob?tabs=microsoft-entra-id#permissions).", + content: "To set up the required permissions:\n\n1. Navigate to Azure Console > Storage Account > IAM > Add role assignment\n2. Add the following special permissions:\n - Storage Blob Data Contributor\n - Storage Queue Data Contributor\n\n[See the documentation](https://learn.microsoft.com/en-us/rest/api/storageservices/put-blob?tabs=microsoft-entra-id#permissions) for more details.",
61-68: Consider making metadata headers configurable.The headers include hardcoded metadata values (
x-ms-meta-m1: v1,x-ms-meta-m2: v2). Consider making these configurable through props.+ metadata: { + type: "object", + label: "Metadata", + description: "Custom metadata key-value pairs to set on the blob.", + optional: true, + },Then in the run method:
headers: { "x-ms-blob-type": "BlockBlob", "Content-Type": "text/plain; charset=UTF-8", "x-ms-blob-content-disposition": `attachment; filename=${fileName}`, - "x-ms-meta-m1": "v1", - "x-ms-meta-m2": "v2", "Content-Length": data?.length, + ...(this.metadata && Object.entries(this.metadata).reduce((acc, [key, value]) => ({ + ...acc, + [`x-ms-meta-${key}`]: value, + }), {})), },
📜 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 (14)
components/azure_storage/actions/create-container/create-container.mjs(1 hunks)components/azure_storage/actions/delete-blob/delete-blob.mjs(1 hunks)components/azure_storage/actions/upload-blob/upload-blob.mjs(1 hunks)components/azure_storage/azure_storage.app.mjs(1 hunks)components/azure_storage/common/constants.mjs(1 hunks)components/azure_storage/common/utils.mjs(1 hunks)components/azure_storage/package.json(2 hunks)components/azure_storage/sources/blob-deleted/blob-deleted.mjs(1 hunks)components/azure_storage/sources/blob-deleted/test-event.mjs(1 hunks)components/azure_storage/sources/common/polling.mjs(1 hunks)components/azure_storage/sources/new-blob-created/new-blob-created.mjs(1 hunks)components/azure_storage/sources/new-blob-created/test-event.mjs(1 hunks)components/azure_storage/sources/new-container-created/new-container-created.mjs(1 hunks)components/azure_storage/sources/new-container-created/test-event.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/azure_storage/sources/new-blob-created/test-event.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (13)
components/azure_storage/azure_storage.app.mjs (7)
1-4: Imports look good.
These dependencies are essential for XML parsing and HTTP requests. No issues spotted here.
5-9: Confirm arrayMode configuration.
SettingarrayMode: truecauses all XML nodes to parse as arrays. Ensure this is the intended behavior to simplify subsequent parsing logic.
52-59: Verify authorization approach.
Using a direct OAuth Bearer token with Azure Blob Storage is atypical. Usually, a shared access signature (SAS) or shared key is used. Confirm your OAuth credentials are valid for these calls.
83-88: PUT method logic is succinct.
No issues identified. This wrapper clearly delegates to_makeRequest.
89-94: DELETE method name is acceptable.
While naming a methoddeletecan be confusing in some contexts, here it’s contained and unambiguous. Implementation looks good.
95-104: listContainers usage is correct.
Passingcomp: "list"is standard for enumerating Azure containers.
105-117: listBlobs method is consistent with Azure standards.
Settingrestype=containerandcomp=listis the correct approach for listing blobs.components/azure_storage/common/constants.mjs (1)
1-9: Confirm the API version.
"2025-01-05" may not be a valid or officially supported Azure Storage API version. Verify if this version is recognized or if a stable version like "2021-12-02" should be used.✅ Verification successful
🌐 Web query:
Is "2025-01-05" a valid Azure Storage API version?💡 Result:
Yes, "2025-01-05" is a valid Azure Storage API version based on the evidence from multiple sources:
Current recommended version:
Microsoft explicitly states in their documentation that 2025-01-05 is the current Azure Storage service version as of October 2024[3][7]. This version includes features like:
- Provisioned v2 billing model support for Azure Files[3][4]
- Binary format specification for file permissions during copy operations[3]
SDK/Client Library Support:
The azure-storage-file-share Python package added support for this version in its 12.20.0b1 release (October 2024)[2].Tooling Compatibility:
Azurite (Azure Storage emulator) specifically added support for 2025-01-05 in its November 2024 release[6][10].Feature Documentation:
Multiple REST API endpoints like:
Create Share[4]Get Share Properties[8]
explicitly mention 2025-01-05 as a supported version.The documentation recommends always using the latest version (2025-01-05) to benefit from performance optimizations and new features[1][3].
Citations:
- 1: https://learn.microsoft.com/sl-si/rest/api/storageservices/versioning-best-practices
- 2: https://pypi.org/project/azure-storage-file-share/
- 3: https://learn.microsoft.com/en-us/rest/api/storageservices/Versioning-for-the-Azure-Storage-Services?redirectedfrom=MSDN
- 4: https://learn.microsoft.com/en-us/rest/api/storageservices/create-share
- 5: https://learn.microsoft.com/en-us/azure/api-management/api-management-versions
- 6: https://github.com/Azure/Azurite/actions
- 7: https://learn.microsoft.com/en-us/rest/api/storageservices/previous-azure-storage-service-versions
- 8: https://learn.microsoft.com/en-us/rest/api/storageservices/get-share-properties
- 9: https://learn.microsoft.com/en-us/rest/api/storageservices/version-2021-06-08
- 10: https://github.com/azure/azurite/releases
API Version is Valid
The "2025-01-05" version is confirmed as a valid Azure Storage API version as per the latest Microsoft documentation and associated sources. No changes are needed.
components/azure_storage/sources/new-container-created/test-event.mjs (1)
1-14: Test event structure is clearly defined.
This static object is fine for testing, but ensure it remains updated if the production schema changes.components/azure_storage/sources/blob-deleted/blob-deleted.mjs (1)
48-54: Maintain consistency with new-blob-created timestamp format.For consistency with the previous recommendation, the timestamp should use ISO format here as well.
components/azure_storage/actions/upload-blob/upload-blob.mjs (1)
1-9: LGTM! Component metadata and imports are well-structured.The component is well-documented with clear name, description, and version. The documentation link is properly included.
components/azure_storage/package.json (2)
3-3: Version bump looks appropriate.The version bump from 0.0.1 to 0.1.0 aligns with the addition of new features.
15-17: Verify the platform dependency version.Please ensure that version 3.0.3 of @pipedream/platform is the latest stable version.
✅ Verification successful
🌐 Web query:
What is the latest version of @pipedream/platform?💡 Result:
The latest version of
@pipedream/platformis 3.0.3, as confirmed by its npm registry entry updated in September 2024[6][8]. This version includes type definitions and runtime type checking for Pipedream platform globals, with usage examples available in its documentation[6][8]. For security and compatibility, always verify you’re using this version vianpm update @pipedream/platform.Citations:
- 1: https://pipedream.com/community/t/how-to-update-a-custom-action-to-use-the-latest-version/9951
- 2: https://pipedream.com/docs/migrate-from-v1
- 3: https://envoy.com/integrations/pipedream
- 4: https://pipedream.com
- 5: https://github.com/boltops-tools/pipedream/blob/master/CHANGELOG.md
- 6: https://www.npmjs.com/package/@pipedream/platform?activeTab=dependencies
- 7: https://pipedream.com/community/t/how-to-inspect-the-version-history-for-a-given-step/8561
- 8: https://www.npmjs.com/package/@pipedream/platform
- 9: https://pipedream.com/docs
- 10: https://github.com/PipedreamHQ/pipedream?via=cagri&tab=readme-ov-file
@pipedream/platform Version Verified
The npm registry confirms that version 3.0.3 is the latest stable release, so the dependency in your package.json is correct.
components/azure_storage/sources/new-container-created/new-container-created.mjs
Outdated
Show resolved
Hide resolved
jcortes
force-pushed
the
azure-storage-new-components
branch
from
212e8c3 to
911b4e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
components/azure_storage/sources/blob-deleted/blob-deleted.mjs (1)
34-36:⚠️ Potential issueStrengthen deleted status check.
The current implementation might miss edge cases where
resourceis null/undefined.Apply this diff to add explicit type checking:
isResourceRelevant(resource) { - return resource?.Deleted === true; + return Boolean(resource) && resource.Deleted === true; },components/azure_storage/azure_storage.app.mjs (1)
85-87:⚠️ Potential issueFix XML parsing of response data.
The current implementation attempts to parse the entire response object instead of its data property.
Apply this diff to fix the parsing:
return jsonOutput - ? parser.parse(response) + ? parser.parse(response.data) : response;
🧹 Nitpick comments (1)
components/azure_storage/sources/blob-deleted/test-event.mjs (1)
5-6: Update test event timestamps to use realistic dates.The test event uses future dates (Feb 2025) for timestamps. This could cause confusion or test failures when the dates become past dates.
Use current or past dates relative to the current time:
- "Creation-Time": "Tue, 11 Feb 2025 23:05:34 GMT", - "Last-Modified": "Tue, 11 Feb 2025 23:05:34 GMT", + "Creation-Time": "Tue, 11 Feb 2024 23:05:34 GMT", + "Last-Modified": "Tue, 11 Feb 2024 23:05:34 GMT", - "DeletedTime": "Tue, 11 Feb 2025 23:12:16 GMT", + "DeletedTime": "Tue, 11 Feb 2024 23:12:16 GMT",Also applies to: 20-20
📜 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 (14)
components/azure_storage/actions/create-container/create-container.mjs(1 hunks)components/azure_storage/actions/delete-blob/delete-blob.mjs(1 hunks)components/azure_storage/actions/upload-blob/upload-blob.mjs(1 hunks)components/azure_storage/azure_storage.app.mjs(1 hunks)components/azure_storage/common/constants.mjs(1 hunks)components/azure_storage/common/utils.mjs(1 hunks)components/azure_storage/package.json(2 hunks)components/azure_storage/sources/blob-deleted/blob-deleted.mjs(1 hunks)components/azure_storage/sources/blob-deleted/test-event.mjs(1 hunks)components/azure_storage/sources/common/polling.mjs(1 hunks)components/azure_storage/sources/new-blob-created/new-blob-created.mjs(1 hunks)components/azure_storage/sources/new-blob-created/test-event.mjs(1 hunks)components/azure_storage/sources/new-container-created/new-container-created.mjs(1 hunks)components/azure_storage/sources/new-container-created/test-event.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- components/azure_storage/sources/new-container-created/test-event.mjs
- components/azure_storage/package.json
- components/azure_storage/common/constants.mjs
- components/azure_storage/sources/new-blob-created/test-event.mjs
- components/azure_storage/actions/delete-blob/delete-blob.mjs
- components/azure_storage/common/utils.mjs
- components/azure_storage/sources/new-container-created/new-container-created.mjs
- components/azure_storage/sources/common/polling.mjs
- components/azure_storage/actions/create-container/create-container.mjs
- components/azure_storage/sources/new-blob-created/new-blob-created.mjs
🧰 Additional context used
🪛 GitHub Check: Lint Code Base
components/azure_storage/sources/blob-deleted/blob-deleted.mjs
[warning] 7-7:
Source names should start with "New". See https://pipedream.com/docs/components/guidelines/#source-name
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
jcortes
force-pushed
the
azure-storage-new-components
branch
from
911b4e3 to
e0a2a42
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
components/azure_storage/azure_storage.app.mjs (2)
58-65:⚠️ Potential issueAdd OAuth token validation.
The authorization header is set without validating the OAuth token.
getHeaders(headers) { return { ...headers, "x-ms-date": new Date().toUTCString(), "x-ms-version": constants.API_VERSION, - "Authorization": `Bearer ${this.$auth.oauth_access_token}`, + "Authorization": this.$auth.oauth_access_token + ? `Bearer ${this.$auth.oauth_access_token}` + : throw new Error("OAuth token is required for authentication"), }; },
66-88:⚠️ Potential issueFix XML parsing of response data.
The response parsing is incorrect as it parses the entire response object instead of its data.
return jsonOutput - ? parser.parse(response) + ? parser.parse(response.data) : response;components/azure_storage/actions/delete-blob/delete-blob.mjs (2)
34-40:⚠️ Potential issueAdd path validation and error handling in deleteBlob method.
The path construction lacks validation for special characters and spaces in blob/container names.
deleteBlob({ containerName, blobName, } = {}) { + if (!containerName || !blobName) { + throw new Error('Container name and blob name are required'); + } + const encodedPath = `/${encodeURIComponent(containerName)}/${encodeURIComponent(blobName)}`; return this.app.delete({ - path: `/${containerName}/${blobName}`, + path: encodedPath, }); },
42-59:⚠️ Potential issueImprove error handling and response in run method.
The method should provide more detailed error information and handle specific error cases.
async run({ $ }) { + try { const { deleteBlob, containerName, blobName, } = this; await deleteBlob({ $, containerName, blobName, }); $.export("$summary", "Successfully deleted blob."); return { success: true, + containerName, + blobName, }; + } catch (error) { + $.export("$summary", `Failed to delete blob: ${error.message}`); + throw error; + } },
🧹 Nitpick comments (2)
components/azure_storage/azure_storage.app.mjs (1)
5-9: Consider refining parser configuration for better error handling.The current parser configuration might need additional options for robustness:
- Add
isArrayfunction to handle array detection more precisely- Add
parseAttributeValueto handle boolean/number attributesconst parser = new XMLParser({ ignoreAttributes: false, arrayMode: true, textNodeName: "value", + isArray: (name) => ["Container", "Blob"].includes(name), + parseAttributeValue: true, });components/azure_storage/actions/delete-blob/delete-blob.mjs (1)
11-16: Improve eslint configuration and info alert formatting.
- The eslint-disable comment should be more specific to the rules being disabled.
- The info alert content could be formatted better for readability.
- // eslint-disable-next-line pipedream/props-label, pipedream/props-description + /* eslint-disable-next-line pipedream/props-label, pipedream/props-description */ info: { type: "alert", alertType: "info", - content: "In order to have the right permissions to use this feature you need to go to the Azure Console in `Storage Account > IAM > Add role assignment`, and add the special permissions for this type of request:\n - Storage Blob Data Contributor\n - Storage Queue Data Contributor\n [See the documentation](https://learn.microsoft.com/en-us/rest/api/storageservices/delete-blob?tabs=microsoft-entra-id#permissions).", + content: [ + "To set up the required permissions:", + "1. Navigate to Azure Console > Storage Account > IAM > Add role assignment", + "2. Add the following special permissions:", + " - Storage Blob Data Contributor", + " - Storage Queue Data Contributor", + "", + "[See the documentation](https://learn.microsoft.com/en-us/rest/api/storageservices/delete-blob?tabs=microsoft-entra-id#permissions)", + ].join("\n"), },
📜 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 (14)
components/azure_storage/actions/create-container/create-container.mjs(1 hunks)components/azure_storage/actions/delete-blob/delete-blob.mjs(1 hunks)components/azure_storage/actions/upload-blob/upload-blob.mjs(1 hunks)components/azure_storage/azure_storage.app.mjs(1 hunks)components/azure_storage/common/constants.mjs(1 hunks)components/azure_storage/common/utils.mjs(1 hunks)components/azure_storage/package.json(2 hunks)components/azure_storage/sources/blob-deleted/blob-deleted.mjs(1 hunks)components/azure_storage/sources/blob-deleted/test-event.mjs(1 hunks)components/azure_storage/sources/common/polling.mjs(1 hunks)components/azure_storage/sources/new-blob-created/new-blob-created.mjs(1 hunks)components/azure_storage/sources/new-blob-created/test-event.mjs(1 hunks)components/azure_storage/sources/new-container-created/new-container-created.mjs(1 hunks)components/azure_storage/sources/new-container-created/test-event.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- components/azure_storage/sources/blob-deleted/test-event.mjs
- components/azure_storage/sources/new-blob-created/test-event.mjs
- components/azure_storage/actions/create-container/create-container.mjs
- components/azure_storage/sources/new-container-created/new-container-created.mjs
- components/azure_storage/sources/common/polling.mjs
- components/azure_storage/common/constants.mjs
- components/azure_storage/sources/blob-deleted/blob-deleted.mjs
- components/azure_storage/package.json
- components/azure_storage/sources/new-container-created/test-event.mjs
- components/azure_storage/actions/upload-blob/upload-blob.mjs
- components/azure_storage/sources/new-blob-created/new-blob-created.mjs
- components/azure_storage/common/utils.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
components/azure_storage/azure_storage.app.mjs (2)
14-50: Well-structured property definitions with proper null checks!The implementation includes:
- Clear type definitions and descriptions
- Proper null checks before accessing nested properties
- Consistent handling of single vs array responses
89-123: LGTM: Well-structured CRUD operations!The implementation of PUT, DELETE, and LIST operations is clean and consistent:
- Proper method encapsulation
- Consistent parameter handling
- Clear separation of concerns
components/azure_storage/actions/delete-blob/delete-blob.mjs (1)
1-8: LGTM! Well-documented component setup.The component is properly configured with clear documentation and version information.
jcortes
force-pushed
the
azure-storage-new-components
branch
from
e0a2a42 to
4d8d396
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
components/azure_storage/common/utils.mjs (2)
4-14:⚠️ Potential issueAdd path traversal protection to
checkTmp.The function should prevent path traversal attacks by validating that the final path does not escape the
/tmpdirectory.Apply this diff to add path traversal protection:
function checkTmp(filePath) { + const normalizedPath = path.normalize(filePath); + if (normalizedPath.includes('..')) { + throw new ConfigurationError("Path traversal is not allowed!"); + } + const adjustedPath = filePath.startsWith("/tmp") ? filePath : `/tmp/${filePath}`; if (!fs.existsSync(adjustedPath)) { throw new ConfigurationError("File does not exist!"); } return adjustedPath; }
16-20:⚠️ Potential issueImprove error handling and performance in
getDataFromFile.The function has two issues:
- Uses synchronous file reading which can block the event loop
- Missing error handling for file read operation
Apply this diff to improve error handling and make the operation asynchronous:
-function getDataFromFile(filePath) { +async function getDataFromFile(filePath) { const path = checkTmp(filePath); - const file = fs.readFileSync(path); - return file; + try { + const file = await fs.promises.readFile(path); + return file; + } catch (error) { + throw new Error(`Failed to read file: ${error.message}`); + } }
🧹 Nitpick comments (3)
components/azure_storage/sources/blob-deleted/blob-deleted.mjs (1)
49-55: Use blob's deletion time instead of current time.The
generateMetamethod usesDate.now()which might not reflect the actual deletion time of the blob. Consider using the blob'sDeletedtimestamp if available.generateMeta(resource) { return { id: resource.Name, summary: `Blob Deleted: ${resource.Name}`, - ts: Date.now(), + ts: resource.Properties?.['Deleted-Time'] + ? new Date(resource.Properties['Deleted-Time']).getTime() + : Date.now(), }; },components/azure_storage/azure_storage.app.mjs (1)
5-9: Consider moving parser instance inside _makeRequest.Creating a global parser instance might lead to memory leaks in long-running applications. Consider moving the parser creation inside the
_makeRequestmethod where it's used.-const parser = new XMLParser({ - ignoreAttributes: false, - arrayMode: true, - textNodeName: "value", -}); export default { // ... methods: { async _makeRequest({ $ = this, path, headers, jsonOutput = true, ...args } = {}) { + const parser = new XMLParser({ + ignoreAttributes: false, + arrayMode: true, + textNodeName: "value", + }); // ... rest of the method } } }components/azure_storage/package.json (1)
15-18: Consider using exact versions and adding missing dependencies.
- Use exact versions for better reproducibility
- Add
fast-xml-parseras it's used in the app module"dependencies": { - "@pipedream/platform": "^3.0.3", - "mime-types": "^2.1.35" + "@pipedream/platform": "3.0.3", + "fast-xml-parser": "4.3.2", + "mime-types": "2.1.35" }
📜 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 (14)
components/azure_storage/actions/create-container/create-container.mjs(1 hunks)components/azure_storage/actions/delete-blob/delete-blob.mjs(1 hunks)components/azure_storage/actions/upload-blob/upload-blob.mjs(1 hunks)components/azure_storage/azure_storage.app.mjs(1 hunks)components/azure_storage/common/constants.mjs(1 hunks)components/azure_storage/common/utils.mjs(1 hunks)components/azure_storage/package.json(2 hunks)components/azure_storage/sources/blob-deleted/blob-deleted.mjs(1 hunks)components/azure_storage/sources/blob-deleted/test-event.mjs(1 hunks)components/azure_storage/sources/common/polling.mjs(1 hunks)components/azure_storage/sources/new-blob-created/new-blob-created.mjs(1 hunks)components/azure_storage/sources/new-blob-created/test-event.mjs(1 hunks)components/azure_storage/sources/new-container-created/new-container-created.mjs(1 hunks)components/azure_storage/sources/new-container-created/test-event.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- components/azure_storage/sources/new-container-created/test-event.mjs
- components/azure_storage/sources/new-blob-created/test-event.mjs
- components/azure_storage/actions/upload-blob/upload-blob.mjs
- components/azure_storage/sources/blob-deleted/test-event.mjs
- components/azure_storage/actions/delete-blob/delete-blob.mjs
- components/azure_storage/common/constants.mjs
- components/azure_storage/sources/common/polling.mjs
- components/azure_storage/sources/new-blob-created/new-blob-created.mjs
- components/azure_storage/actions/create-container/create-container.mjs
- components/azure_storage/sources/new-container-created/new-container-created.mjs
🧰 Additional context used
🪛 GitHub Check: Lint Code Base
components/azure_storage/sources/blob-deleted/blob-deleted.mjs
[warning] 7-7:
Source names should start with "New". See https://pipedream.com/docs/components/guidelines/#source-name
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (2)
components/azure_storage/sources/blob-deleted/blob-deleted.mjs (2)
34-36: Strengthen deleted status check.The current implementation might miss edge cases where
resourceis null/undefined.isResourceRelevant(resource) { - return resource?.Deleted === true; + return Boolean(resource) && resource.Deleted === true; },
6-8:✅ Verification successful
Update source name to follow guidelines.
According to the Pipedream documentation, source names should start with "New". Consider renaming to "New Blob Deleted".
🌐 Web query:
What is the naming convention for Pipedream source components?💡 Result:
Pipedream source components follow specific naming conventions to ensure clarity and consistency across their registry. According to the guidelines:
Source names should:
- Be singular and title-cased (e.g., "New File" instead of "New Files")
- Start with "New" when emitting events about newly created/added items (e.g., "New Shared Drive")
- Avoid including app names redundantly if already part of their
keyproperty- Not be slugified (e.g., use spaces instead of hyphens)
Examples from official sources:
New Shared Drive(for Google Drive)[2]New Files (Instant)(Google Drive)[3]New Order(Shopify example from video content)[7]For component keys in source code:
- Use past-tense verbs describing events (e.g.,
linear_app-issue-created-instant)[2]- Follow slugified patterns like
app_name_slug-slugified-component-name[2]These conventions help users quickly identify component functionality while maintaining consistency across Pipedream's ecosystem[2][3][7].
Citations:
- 1: https://pipedream.com/docs/components/api
- 2: https://pipedream.com/docs/components/guidelines
- 3: https://pipedream.com/docs/components
- 4: https://pipedream.com/docs/components/typescript
- 5: https://pipedream.com/docs/cli/reference
- 6: https://www.jsdelivr.com/package/npm/eslint-plugin-pipedream
- 7: https://www.youtube.com/watch?v=K5FrIqm-03k
- 8: https://www.youtube.com/watch?v=7o-Tk1CPkvw
- 9: https://github.com/PipedreamHQ/pipedream/blob/master/docs-v2/pages/glossary.mdx
- 10: https://pipedream.com/docs/components/sources-quickstart
Action Required: Update Source Name to Conform to Guidelines
Based on the Pipedream naming conventions, source names must be singular, title-cased, and prefixed with “New” when appropriate. Please update the source name in
components/azure_storage/sources/blob-deleted/blob-deleted.mjsfrom:name: "Blob Deleted",to
name: "New Blob Deleted",This change will ensure consistency with the guidelines outlined in the Pipedream documentation.
🧰 Tools
🪛 GitHub Check: Lint Code Base
[warning] 7-7:
Source names should start with "New". See https://pipedream.com/docs/components/guidelines/#source-name
| function getFilenameFromPath(filePath) { | ||
| const pathParts = filePath.split("/"); | ||
| return pathParts[pathParts.length - 1]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use path.basename for robust filename extraction.
The current implementation may fail with Windows-style paths or paths with trailing slashes. Use Node's built-in path.basename for more robust filename extraction.
Apply this diff to improve the implementation:
+import path from "path";
+
function getFilenameFromPath(filePath) {
- const pathParts = filePath.split("/");
- return pathParts[pathParts.length - 1];
+ return path.basename(filePath);
}📝 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.
| function getFilenameFromPath(filePath) { | |
| const pathParts = filePath.split("/"); | |
| return pathParts[pathParts.length - 1]; | |
| } | |
| import path from "path"; | |
| function getFilenameFromPath(filePath) { | |
| return path.basename(filePath); | |
| } |
| getResourcesFromResponse(response) { | ||
| const resources = response.EnumerationResults.Blobs.Blob; | ||
| if (!resources) { | ||
| return []; | ||
| } | ||
| return Array.isArray(resources) | ||
| ? resources | ||
| : [ | ||
| resources, | ||
| ]; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for XML parsing.
The getResourcesFromResponse method should handle cases where EnumerationResults or Blobs might be undefined.
getResourcesFromResponse(response) {
+ if (!response?.EnumerationResults?.Blobs) {
+ return [];
+ }
const resources = response.EnumerationResults.Blobs.Blob;
if (!resources) {
return [];
}
return Array.isArray(resources)
? resources
: [
resources,
];
},📝 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.
| getResourcesFromResponse(response) { | |
| const resources = response.EnumerationResults.Blobs.Blob; | |
| if (!resources) { | |
| return []; | |
| } | |
| return Array.isArray(resources) | |
| ? resources | |
| : [ | |
| resources, | |
| ]; | |
| }, | |
| getResourcesFromResponse(response) { | |
| if (!response?.EnumerationResults?.Blobs) { | |
| return []; | |
| } | |
| const resources = response.EnumerationResults.Blobs.Blob; | |
| if (!resources) { | |
| return []; | |
| } | |
| return Array.isArray(resources) | |
| ? resources | |
| : [ | |
| resources, | |
| ]; | |
| }, |
| async _makeRequest({ | ||
| $ = this, path, headers, jsonOutput = true, ...args | ||
| } = {}) { | ||
| let response; | ||
| try { | ||
| response = await axios($, { | ||
| ...args, | ||
| url: this.getUrl(path), | ||
| headers: this.getHeaders(headers), | ||
| }); | ||
|
|
||
| } catch (error) { | ||
| const errorResponse = parser.parse(error.response.data); | ||
| if (errorResponse.Error) { | ||
| throw new Error(JSON.stringify(errorResponse.Error, null, 2)); | ||
| } | ||
| throw error; | ||
| } | ||
|
|
||
| return jsonOutput | ||
| ? parser.parse(response) | ||
| : response; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add rate limiting and retry logic.
The _makeRequest method should implement rate limiting and retry logic for transient failures. Consider using exponential backoff for retries.
async _makeRequest({
$ = this, path, headers, jsonOutput = true, ...args
} = {}) {
let response;
+ const maxRetries = 3;
+ const baseDelay = 1000;
+
+ for (let attempt = 0; attempt < maxRetries; attempt++) {
try {
response = await axios($, {
...args,
url: this.getUrl(path),
headers: this.getHeaders(headers),
});
+ break;
} catch (error) {
+ if (error.response?.status === 429 ||
+ (error.response?.status >= 500 && error.response?.status < 600)) {
+ if (attempt === maxRetries - 1) throw error;
+ await new Promise(resolve =>
+ setTimeout(resolve, baseDelay * Math.pow(2, attempt)));
+ continue;
+ }
const errorResponse = parser.parse(error.response.data);
if (errorResponse.Error) {
throw new Error(JSON.stringify(errorResponse.Error, null, 2));
}
throw error;
}
+ }
return jsonOutput
? parser.parse(response)
: response;
},📝 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 _makeRequest({ | |
| $ = this, path, headers, jsonOutput = true, ...args | |
| } = {}) { | |
| let response; | |
| try { | |
| response = await axios($, { | |
| ...args, | |
| url: this.getUrl(path), | |
| headers: this.getHeaders(headers), | |
| }); | |
| } catch (error) { | |
| const errorResponse = parser.parse(error.response.data); | |
| if (errorResponse.Error) { | |
| throw new Error(JSON.stringify(errorResponse.Error, null, 2)); | |
| } | |
| throw error; | |
| } | |
| return jsonOutput | |
| ? parser.parse(response) | |
| : response; | |
| }, | |
| async _makeRequest({ | |
| $ = this, path, headers, jsonOutput = true, ...args | |
| } = {}) { | |
| let response; | |
| const maxRetries = 3; | |
| const baseDelay = 1000; | |
| for (let attempt = 0; attempt < maxRetries; attempt++) { | |
| try { | |
| response = await axios($, { | |
| ...args, | |
| url: this.getUrl(path), | |
| headers: this.getHeaders(headers), | |
| }); | |
| break; | |
| } catch (error) { | |
| if ( | |
| error.response?.status === 429 || | |
| (error.response?.status >= 500 && error.response?.status < 600) | |
| ) { | |
| if (attempt === maxRetries - 1) throw error; | |
| await new Promise((resolve) => | |
| setTimeout(resolve, baseDelay * Math.pow(2, attempt)) | |
| ); | |
| continue; | |
| } | |
| const errorResponse = parser.parse(error.response.data); | |
| if (errorResponse.Error) { | |
| throw new Error(JSON.stringify(errorResponse.Error, null, 2)); | |
| } | |
| throw error; | |
| } | |
| } | |
| return jsonOutput ? parser.parse(response) : response; | |
| }, |
WHY
Resolves #15177
Summary by CodeRabbit
New Features
Chores