[Components] google_drive - Add raw file response possibility #17367
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
@jocarino is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe Google Drive "download file" action was updated to support an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DownloadAction
participant GoogleDriveAPI
participant FileSystem
User->>DownloadAction: Trigger download-file action (with/without getBufferResponse)
DownloadAction->>GoogleDriveAPI: Request file content stream
GoogleDriveAPI-->>DownloadAction: Return file content stream
alt getBufferResponse is true
DownloadAction->>DownloadAction: Read stream fully into buffer
DownloadAction-->>User: Return buffer and metadata with summary
else getBufferResponse is false or unset
DownloadAction->>FileSystem: Write stream to filePath (default /tmp/)
FileSystem-->>DownloadAction: Confirm file write
DownloadAction-->>User: Return filePath and metadata with summary
end
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/google_drive/actions/download-file/download-file.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/google_drive/actions/download-file/download-file.mjs (2)
128-135: LGTM! Clean raw response implementation.The raw response logic correctly returns file content and metadata directly when enabled. The implementation is clean and follows the component's patterns.
Consider documenting potential memory usage implications for large files when using raw response mode.
128-135: LGTM: Clean implementation of raw response functionality.The logic correctly handles the raw response case with appropriate summary messaging. Consider documenting potential memory implications for large files when using this option.
Consider adding a note in the
getRawResponsedescription about potential memory usage for large files:- description: "Whether to return the file content directly in the response instead of writing to a file path", + description: "Whether to return the file content directly in the response instead of writing to a file path. Note: Large files may consume significant memory.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/google_drive/actions/download-file/download-file.mjs(4 hunks)components/google_drive/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
components/google_drive/package.json (1)
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (12)
components/google_drive/package.json (2)
3-3: LGTM! Appropriate version increment.The patch version bump from 1.0.1 to 1.0.2 correctly follows semantic versioning for the new raw response functionality.
3-3: LGTM: Appropriate version bump for new functionality.The patch version increment correctly reflects the addition of the new
getRawResponsefeature in the download-file action.components/google_drive/actions/download-file/download-file.mjs (10)
21-21: LGTM! Appropriate version increment.The version increment from 0.1.10 to 0.1.11 is consistent with the package-level version bump and reflects the new functionality.
50-50: LGTM! Appropriate property modification.Making
filePathoptional is correct since it's not required when using the raw response feature.
92-97: LGTM! Well-defined new property.The new
getRawResponseproperty is clearly documented and follows the component's property definition pattern.
100-104: LGTM! Proper validation logic.The validation correctly ensures
filePathis provided when not using raw response, with a clear error message.
143-147: LGTM! Maintains backward compatibility.The return structure preserves the existing behavior for file path mode while supporting the new raw response functionality.
21-21: LGTM: Version increment follows standard practice.The minor version bump correctly reflects the new functionality being added.
50-50: LGTM: Making filePath optional aligns with new raw response functionality.This change properly supports the new use case where files can be returned as raw content without requiring a file path.
92-97: LGTM: Well-defined getRawResponse property.The new boolean property is properly structured with clear label and description that explains its purpose.
100-104: LGTM: Excellent validation logic maintaining backwards compatibility.The validation correctly ensures that existing implementations continue to work while enabling the new raw response functionality. The error message is clear and helpful.
143-146: LGTM: Return statement maintains consistency and backwards compatibility.The modified return structure preserves the existing API contract while cleanly separating the two execution paths.
andrewjschuang
moved this to Ready for PR Review
in Component (Source and Action) Backlog
jcortes
moved this from Ready for PR Review
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to In QA
in Component (Source and Action) Backlog
|
Hi @jocarino, could you share what is your usecase to use the raw file content? In Pipedream step, the output is serialized into JSON object, which is not suitable for file content (which is usually binary). This is why we used the tmp dir to store the file to be used for subsequence steps. |
vunguyenhung
moved this from In QA
to Changes Required
in Component (Source and Action) Backlog
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check the test report below for more information |
|
Hi @jcortes and @vunguyenhung |
jcortes
left a comment
jcortes
left a comment
There was a problem hiding this comment.
Please change the version of the component! Thanks!
components/google_drive/actions/download-file/download-file.mjs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jorge Cortes <[email protected]>
|
@jcortes @vunguyenhung let me know if you need anything else from me? |
vunguyenhung
moved this from Changes Required
to Ready for PR Review
in Component (Source and Action) Backlog
|
Thanks @jocarino, I've moved the PR to the next phase. In the mean time, could you please resolve the code conflict? |
|
just noticed it, fixing it |
michelle0927
moved this from Ready for PR Review
to In Review
in Component (Source and Action) Backlog
michelle0927
left a comment
michelle0927
left a comment
There was a problem hiding this comment.
LGTM! Ready for QA!
michelle0927
moved this from In Review
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to In QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from In QA
to Ready for Release
in Component (Source and Action) Backlog
|
Hi everyone, all test cases are passed! Ready for release! |
|
Merge this on behalf of the user. |
github-project-automation
bot
moved this from Ready for Release
to Done
in Component (Source and Action) Backlog
7 participants
WHY
Atm there's no option to the the downloaded file directly, only from the pipeline tmp folder -> only works with workflows.
I've added the possibility to download the raw response from google drive directly, and turned the temp file path optional, if raw response is true
Summary by CodeRabbit
New Features
Chores