-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Part 3 - applying file URL or path improvement #17149
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
base: master
Are you sure you want to change the base?
Conversation
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: 19
🔭 Outside diff range comments (6)
components/scrapfly/actions/ai-data-extraction/ai-data-extraction.mjs (1)
77-81
: Avoid sending anundefined
Content-Type headerIf
this.contentType
is not supplied the request currently setscontent-type: undefined
, which many servers interpret literally. Construct the header only when a value exists.-headers: { - "content-type": this.contentType, -}, +headers: this.contentType + ? { "content-type": this.contentType } + : undefined,components/zoho_catalyst/actions/perform-face-detection-and-analysis/perform-face-detection-and-analysis.ts (1)
61-71
: Falsy (but valid) values are silently ignored when constructing the payload
if (this[prop])
filters out everything that is falsy.
That means:
mode: 0
– perfectly valid numeric value – is dropped.- Explicit
false
for the boolean flags is never sent (the server will fall back to its own default, which might betrue
).- if (this[prop]) { - data.append(prop, this[prop].toString()); - } + if (this[prop] !== undefined && this[prop] !== null) { + data.append(prop, String(this[prop])); + }Ensuring we test only for
undefined
/null
preserves intentional0
orfalse
inputs.components/sonix/actions/upload-media/upload-media.mjs (1)
39-43
:transcriptText
is typed as boolean but sent as textThe Sonix API expects the full transcript string under
transcript_text
, not a boolean flag. Current definition will always coerce to"true"
/"false"
.- transcriptText: { - type: "boolean", - label: "Transcript Text", - description: "Existing transcript - if present, will align the transcript rather than transcribing.", - optional: true, - }, + transcriptText: { + type: "string", + label: "Transcript Text", + description: "Existing transcript text to align rather than transcribe.", + optional: true, + },And remove the boolean coercion in
run()
:- this.transcriptText && formData.append("transcript_text", `${this.transcriptText}`); + this.transcriptText && formData.append("transcript_text", this.transcriptText);components/roboflow/roboflow.app.mjs (1)
38-48
: RequiredfilePath
+ still-optionalfileUrl
creates conflicting API.
filePath
lost itsoptional: true
, so any action that only wants a URL now forces users to supply an unused path.
Consider either:
- Marking
filePath
optional again, or- Removing
fileUrl
entirely and updating downstream actions to rely solely onfilePath
.Failing to align the two will surface validation errors in components that still expect the old behaviour.
components/printify/actions/create-product/create-product.mjs (1)
146-154
: Guard against undefined or missing variant data
variantPrice_${i}
andvariantEnabled_${i}
are optional in the UI but you always forward them to the Printify API. When either value isundefined
/null
, the request payload may be rejected.variants.push({ id: this[`variant_${i}`], - price: this[`variantPrice_${i}`], - is_enabled: this[`variantEnabled_${i}`], + ...(this[`variantPrice_${i}`] != null && { price: this[`variantPrice_${i}`] }), + ...(this[`variantEnabled_${i}`] != null && { is_enabled: this[`variantEnabled_${i}`] }), });components/printnode/actions/send-print-job/send-print-job.mjs (1)
60-66
:authentication
prop is now orphanedSince
filePath
(path / URL) is fetched throughgetFileStream
, any HTTP authentication info is silently ignored. Either wire the credentials intogetFileStream
or drop the prop to avoid misleading users.
🧹 Nitpick comments (46)
components/signaturit/package.json (1)
3-3
: Bump package version to 0.2.0.
Ensure you’ve added a corresponding entry in CHANGELOG.md or release notes for this release.components/sonix/package.json (2)
3-3
: Confirm major version bump for breaking changes
The jump to1.0.0
signals breaking changes from previous0.x
releases. Please update the CHANGELOG and README to document migration steps and any API changes for users.
16-16
: Verify platform dependency upgrade
Bumping@pipedream/platform
to^3.1.0
can introduce breaking changes in the underlying helpers (e.g.,getFileStream
/getFileStreamAndMetadata
). Ensure this component’s integration logic aligns with the new platform API and run full integration tests.components/scrapfly/package.json (1)
3-3
: Remember to update CHANGELOG and lock-file when bumping the package versionThe version bump to
0.2.0
looks fine, but please add an entry in the component-level CHANGELOG and run the package-manager (npm i
/pnpm install
) sopackage-lock.json
/pnpm-lock.yaml
remains in sync.components/scrapfly/scrapfly.app.mjs (1)
15-17
: Minor wording tweak to keep description conciseConsider dropping “(for example,
/tmp/myFile.txt
)” or moving the example to the docs to keep the UI tooltip short and consistent with other components.components/scrapfly/actions/ai-data-extraction/ai-data-extraction.mjs (1)
68-90
: Large-file performanceThe action buffers the entire file into memory (
Buffer.concat
). For multi-MB HTML/PDF inputs this may hit the 256 MB memory limit on some deployments. Consider streaming directly to Axios (pass the stream asdata
) and letting Axios handle chunked transfer encoding.components/zoho_catalyst/actions/extract-text-from-image/extract-text-from-image.ts (1)
31-37
: Header handling is fine but watch axios body limits
await getImageFormData(imagePath)
and pipingheaders: data.getHeaders()
is the correctform-data
pattern.
One edge case: axios (the edition bundled by the Pipedream SDK) defaultsmaxBodyLength
/maxContentLength
to 10 MB. Large images will 413. Consider exposing these two axios options via your internal_httpRequest
wrapper or documenting the size limit.components/zoho_catalyst/actions/detect-objects-in-image/detect-objects-in-image.ts (1)
31-37
: Same axios payload-size caveat appliesReplication of the extract-text comment: large images may exceed axios defaults. Centralising the override once in
app._httpRequest
avoids repetition across all image actions.components/zoho_catalyst/actions/perform-image-moderation/perform-image-moderation.ts (1)
40-47
: Minor duplication opportunityEvery image-based action now contains identical:
const data = await getImageFormData(imagePath); ... headers: data.getHeaders(), data,Extracting a small helper (e.g.
this.buildMultipart(imagePath)
) inapp
would DRY this up and keep future header tweaks in one place.components/zoho_catalyst/app/zoho_catalyst.app.ts (1)
29-31
: Clearer prop description – consider input validationGood improvement clarifying URL vs
/tmp
path. To prevent runtime 400s, think about lightweight validation:validate: (v) => v.startsWith("http") || v.startsWith("/tmp/"),(or equivalent) so users get immediate feedback in the UI.
components/zoho_catalyst/actions/perform-face-detection-and-analysis/perform-face-detection-and-analysis.ts (1)
76-77
: Minor: double-check header overwrite behaviour
headers: data.getHeaders()
replaces any default headers yourperformImageFaceDetection
wrapper might already set (e.g. auth, custom UA).
If that wrapper internally merges headers you’re good; otherwise considerheaders: { ...(await this.app.defaultHeaders?.()), ...data.getHeaders(), },so you don’t accidentally drop required headers.
components/zoho_catalyst/common/methods.ts (1)
5-12
: Add graceful fallbacks when metadata is incomplete
metadata.name
,contentType
, orsize
can beundefined
(e.g. some remote URLs don’t exposeContent-Type
orContent-Length
).
Passingundefined
forfilename
,contentType
, orknownLength
makesform-data
throw.- data.append("image", stream, { - filename: metadata.name, - contentType: metadata.contentType, - knownLength: metadata.size, - }); + data.append("image", stream, { + // default to a generic name/type/omit length if missing + filename: metadata.name ?? "file", + ...(metadata.contentType && { contentType: metadata.contentType }), + ...(Number.isFinite(metadata.size) && { knownLength: metadata.size }), + });This keeps the helper robust regardless of the upstream source.
components/zip_archive_api/actions/compress-files/compress-files.mjs (1)
47-55
: Stream files in parallel for better latency
await
-ing eachgetFileStreamAndMetadata
call serially lengthens the setup time for largefiles
arrays. The calls are independent and can safely be performed in parallel:-for (const file of this.files) { - const { stream, metadata } = await getFileStreamAndMetadata(file); - data.append("Files", stream, { - contentType: metadata.contentType, - knownLength: metadata.size, - filename: metadata.name, - }); -} +await Promise.all(this.files.map(async (file) => { + const { stream, metadata } = await getFileStreamAndMetadata(file); + data.append("Files", stream, { + contentType: metadata.contentType, + knownLength: metadata.size, + filename: metadata.name, + }); +}));components/printautopilot/actions/add-pdf-to-queue/add-pdf-to-queue.mjs (1)
14-16
: Prop description should mention size limits / MIME guardrailsNow that remote URLs are accepted, clarify any file-size ceilings imposed by Print Autopilot and whether non-PDF mime types will be rejected. This prevents users from unintentionally streaming multi-hundred-MB files.
components/sonix/actions/upload-media/upload-media.mjs (1)
15-18
:file
prop description references/tmp
, not${TMP_DIR}
To avoid confusion for self-hosters, you may want to phrase it generically (e.g. “path to a file in the ephemeral file system, typically
/tmp
in Pipedream workers”) or expose the TMP env var.components/printnode/package.json (1)
3-16
: Dependency loosened to caret range—lockfile drift riskChanging from pinned
1.5.1
to^3.1.0
introduces non-determinism across installs. Consider using a lockfile or retaining a fixed minor to avoid surprising breakages.components/roboflow/common/utils.mjs (2)
1-1
: Missing explicit type import can hinder treeshaking
getFileStream
is imported as a named export without explicitly type-guarding the returned value. If this module is bundled, omittingtype
information may pull in the full@pipedream/platform
bundle instead of only the stream helper, hurting bundle size.Consider a dynamic import or re-export that limits the imported surface:
-import { getFileStream } from "@pipedream/platform"; +import { getFileStream } from "@pipedream/platform/stream";(if such a sub-path exists in the package).
If not, ignore this suggestion.
21-24
: Default export object grows indefinitelyYou keep appending helpers to the default export object (
extractSubstringAfterSlash
,getBase64File
). Over time this becomes an untyped “kitchen-sink” util.
Export named functions instead to keep the tree-shake-ability and IDE IntelliSense crisp.-export default { - extractSubstringAfterSlash, - getBase64File, -}; +export { extractSubstringAfterSlash, getBase64File };components/scoredetect/package.json (1)
3-3
: Version bump rationaleSame note as Xero: if no user-visible API changed, a patch bump may suffice.
components/sftp/actions/upload-file/upload-file.mjs (1)
34-38
:buffer
variable now carries a stream, not a Buffer – rename for clarity.The ssh2-sftp-client
put()
call accepts both Buffers and streams, so functionality is fine, but continuing to call the variablebuffer
is misleading now that it may be aReadable
.-const buffer = this.file ? - (await getFileStream(this.file)) : - Buffer.from(this.data); +const source = this.file + ? await getFileStream(this.file) // Readable stream + : Buffer.from(this.data); // Buffer ... - buffer: buffer, + buffer: source,components/todoist/actions/import-tasks/import-tasks.mjs (1)
40-45
: Stream is still fully buffered – consider true streaming for large CSVs.Reading the entire file into memory replicates the old behaviour, but for very large CSVs it can blow memory in Lambda-style runtimes.
If Todoist API allows incremental inserts, you could parse rows on-the-fly:-for await (const chunk of stream) { - chunks.push(chunk); -} -const fileContents = Buffer.concat(chunks).toString(); -const tasks = await converter.csv2jsonAsync(fileContents); +for await (const chunk of stream) { + csvParser.write(chunk); +} +csvParser.end(); +const tasks = await collectRows(csvParser);Optional, but worth noting.
components/signaturit/actions/create-certified-email/create-certified-email.mjs (1)
84-92
: Drop strayi++
– it no longer serves a purpose
i
is the recipients counter; increments inside the data block are unrelated and confusing.- i++;
components/zoho_bugtracker/actions/update-bug/update-bug.mjs (1)
255-255
: Nit: capitalisation in summary
ID
is the common acronym; usingId
is inconsistent with other actions.-$.export("$summary", `Successfully updated bug with Id: ${bugId}!`); +$.export("$summary", `Successfully updated bug with ID: ${bugId}!`);components/reform/actions/extract-data-from-document/extract-data-from-document.mjs (1)
30-37
: Provide a fallback content-type & guard unknown length
metadata.contentType
andmetadata.size
are undefined when the remote server does not expose them (common for S3-signed URLs).
Safer to default both values.- formData.append("document", stream, { - filename: metadata.name, - contentType: metadata.contentType, - knownLength: metadata.size, - }); + formData.append("document", stream, { + filename: metadata.name, + contentType: metadata.contentType || "application/octet-stream", + ...(metadata.size ? { knownLength: metadata.size } : {}), + });components/security_reporter/security_reporter.app.mjs (1)
398-405
: Resilient metadata handling & micro-optimisation
- Same fallback issue as above – guard
contentType
/knownLength
.- You iterate files sequentially; a simple
Promise.all
shaves latency with multiple uploads.- const { - stream, metadata, - } = await getFileStreamAndMetadata(path); - data.append("file", stream, { - contentType: metadata.contentType, - knownLength: metadata.size, - filename: metadata.name, - }); + const { stream, metadata } = await getFileStreamAndMetadata(path); + data.append("file", stream, { + filename: metadata.name, + contentType: metadata.contentType || "application/octet-stream", + ...(metadata.size ? { knownLength: metadata.size } : {}), + });Optional concurrency sketch:
await Promise.all(files.map(async (path) => { /* body here */ }));components/ragie/actions/update-document-file/update-document-file.mjs (1)
35-42
: Same metadata fallbacks apply- data.append("file", stream, { - contentType: metadata.contentType, - knownLength: metadata.size, - filename: metadata.name, - }); + data.append("file", stream, { + filename: metadata.name, + contentType: metadata.contentType || "application/octet-stream", + ...(metadata.size ? { knownLength: metadata.size } : {}), + });components/signaturit/actions/create-signature-request-from-template/create-signature-request-from-template.mjs (2)
136-166
: PreferforEach
overmap
when the return value is ignored
arr.map()
returns a new array that is immediately discarded; this is a minor waste and can mislead future readers into thinking the return value matters.- arr.map((item, index) => { - processObject(item, `[${index}]`); - }); + arr.forEach((item, index) => { + processObject(item, `[${index}]`); + });
210-220
: Add a fallback for missingcontentType
and streamline error-handling
metadata.contentType
can beundefined
for local files without an extension or for poorly-served URLs. Several APIs reject such uploads.- formData.append(`files[${k++}]`, stream, { - contentType: metadata.contentType, + formData.append(`files[${k++}]`, stream, { + contentType: metadata.contentType || "application/octet-stream", knownLength: metadata.size, filename: metadata.name, });You may also want to surface a clear error when
metadata
is missing critical fields (e.g.name
) instead of silently passingundefined
.components/sftp/sftp.app.mjs (1)
9-11
: Consider validating the new “path or URL” contractThe prop description now promises support for both URLs and
/tmp
paths, but the helper methods in this app do not consumefile
directly.
Ensure downstream actions that rely onsftp.app
(e.g. upload-file) actually resolve URLs viagetFileStream*
, otherwise users might face runtime errors.components/roboflow/actions/upload-image/upload-image.mjs (1)
34-46
: Surface clearer errors whenfilePath
is neither a valid URL nor a/tmp
path
getFileStreamAndMetadata
will throw, but the user gets a generic stack trace.
Consider pre-validating the input and emitting a friendlier message that explains the accepted formats (URL or/tmp/...
).No code suggestion provided as the preferred UX depends on your error-handling conventions.
components/sendgrid/actions/send-email-single-recipient/send-email-single-recipient.mjs (1)
230-237
: Stream fully loaded into memory – watch out for large filesCollecting all chunks and
Buffer.concat
works but loads the entire attachment into memory.
If users pass very large files this could exhaust the 256 MB memory limit on Pipedream steps. Consider:
- piping the stream through
stream-to-base64
in chunks, or- rejecting files above a safe size (e.g. 10 MB).
components/sendgrid/actions/send-email-multiple-recipients/send-email-multiple-recipients.mjs (1)
213-218
: Memory footprint of full bufferingIdentical concern: entire file buffered in memory; consider streaming or size-guarding.
components/todoist/todoist.app.mjs (1)
256-260
:path
prop is now dual-purpose but still named generically – consider explicit naming or validation
The UI label/description were updated to accept a URL or a/tmp
path, yet the underlying prop key remainspath
. Two follow-ups to think about:
- Ambiguity –
path
is a very generic identifier and clashes with Node’s terminology. A more explicit key such asfilePathOrUrl
(used in other components) would improve readability and autocomplete.- Validation – downstream actions assume the value is valid. A tiny helper that verifies either
•value.startsWith("http")
orvalue.startsWith("/tmp/")
could avoid runtime surprises.Not blocking, but worth aligning with the naming/validation pattern adopted in the other updated components.
components/roboflow/actions/detect-object-from-image/detect-object-from-image.mjs (1)
27-33
: Align prop docs with new URL-or-/tmp
conventionMost actions in this PR now advertise “File Path or URL” to reflect support for
getFileStream…
helpers.
Here thefilePath
prop still inherits the old wording from the app definition, which is inconsistent and will confuse users skimming the UI.Consider overriding the inherited definition locally or updating the shared
filePath
prop so it explicitly mentions that URLs are accepted (ifutils.getBase64File
actually supports them).components/meistertask/actions/create-attachment/create-attachment.mjs (1)
61-68
: Guard against missing metadata fields
metadata.contentType
ormetadata.size
may beundefined
when the remote file lacks
Content-Type
/Content-Length
headers.form-data
will then insert the string
"undefined"
which some servers reject.Add safe fallbacks:
- contentType: metadata.contentType, - knownLength: metadata.size, + contentType: metadata.contentType || "application/octet-stream", + ...(metadata.size && { knownLength: metadata.size }),components/signerx/actions/upload-package/upload-package.mjs (1)
27-34
: Same metadata-null edge case as aboveFor consistency with other actions, add default values for
contentType
and only includeknownLength
when defined.
This prevents runtime errors when uploading from certain URLs.components/security_reporter/actions/create-finding/create-finding.mjs (1)
118-121
: Minor wording: plural label vs. singular description
label: "Draft Documents File"
(singular) but description says “One or more files”.
Consider “Draft Document Files” or “Draft Documents (File upload)” to keep UI wording clear.components/zerobounce/actions/file-validation/file-validation.mjs (1)
76-84
: Fallbacks for unreliable remote headersAs with other actions,
metadata.contentType
andmetadata.size
may be absent.
Passingundefined
triggersform-data
quirks and sometimes invalid multipart parts.formData.append("file", stream, { - contentType: metadata.contentType, - knownLength: metadata.size, - filename: metadata.name, + contentType: metadata.contentType || "application/octet-stream", + ...(metadata.size && { knownLength: metadata.size }), + filename: metadata.name, });components/printify/actions/create-product/create-product.mjs (1)
157-164
: Avoid loading the whole image into memoryConverting the entire stream to a buffer will blow up memory for large artwork. Prefer streaming the file directly to
uploadImage
or at least impose a size limit before concatenation.components/printnode/actions/send-print-job/send-print-job.mjs (1)
72-78
: Streaming entire file into a single buffer may exhaust memoryLarge print jobs (e.g. PDFs) are fully buffered before base64-encoding. Consider a stream-to-base64 pipeline or chunked upload to reduce memory pressure.
components/xero_accounting_api/actions/upload-file/upload-file.mjs (2)
22-26
: Consider extracting the “file input” prop into a shared definition
filePathOrUrl
(string + uniform description) now appears in several components across this PR. Defining it once (e.g. in a commonfileProps.mjs
) and re-using viapropDefinition
would cut duplication and keep wording in sync going forward.
55-63
: Guard against unknown file size before passingknownLength
getFileStreamAndMetadata
may returnmetadata.size = null
when a remote server omits theContent-Length
header. PassingknownLength: null
makesform-data
throw. Include the field only when the size is known:- data.append("file", stream, { - contentType: metadata.contentType, - knownLength: metadata.size, - filename: metadata.name, - }); +const fileOptions = { + contentType: metadata.contentType, + filename: metadata.name, +}; +if (Number.isFinite(metadata.size)) { + fileOptions.knownLength = metadata.size; +} +data.append("file", stream, fileOptions);components/zoho_bugtracker/zoho_bugtracker.app.mjs (1)
276-280
: Fix unbalanced parenthesis in the description stringA missing
)
after “/tmp/myFile.txt
” slips into customer-visible docs:- directory (for example, `/tmp/myFile.txt`. Please configure +directory (for example, `/tmp/myFile.txt`). Please configurecomponents/platerecognizer/actions/run-recognition/run-recognition.mjs (1)
44-50
: Large images are loaded entirely into memory – consider a stream-to-base64 approachFor high-resolution snapshots the
Buffer.concat
strategy can spike memory. A stream-pipeline with a Base64 transform (e.g.import { pipeline } from "stream/promises"; import Base64Encode from "base64-stream";
) would let you encode while streaming and cap the memory footprint.components/pixelbin/actions/upload-file/upload-file.mjs (2)
16-18
: Align prop name with the rest of the codebaseMost components moved to
filePathOrUrl
; this action still exposesfilePath
. Renaming keeps the public API consistent and avoids developer confusion.
83-91
: SameknownLength
caveat as in the Xero uploaderSee earlier comment – skip the
knownLength
option whenmetadata.size
is undefined to avoidform-data
errors for remote files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (94)
components/arpoone/arpoone.app.mjs
(1 hunks)components/meistertask/actions/create-attachment/create-attachment.mjs
(4 hunks)components/meistertask/package.json
(2 hunks)components/offorte/offorte.app.mjs
(1 hunks)components/pdffiller/actions/upload-document/upload-document.mjs
(2 hunks)components/pdffiller/package.json
(2 hunks)components/pdffiller/pdffiller.app.mjs
(1 hunks)components/pembee/pembee.app.mjs
(1 hunks)components/pixelbin/actions/upload-file/upload-file.mjs
(3 hunks)components/pixelbin/package.json
(2 hunks)components/platerecognizer/actions/run-recognition/run-recognition.mjs
(2 hunks)components/platerecognizer/package.json
(2 hunks)components/postmaster/postmaster.app.mjs
(1 hunks)components/printautopilot/actions/add-pdf-to-queue/add-pdf-to-queue.mjs
(2 hunks)components/printautopilot/package.json
(2 hunks)components/printify/actions/create-product/create-product.mjs
(3 hunks)components/printify/package.json
(2 hunks)components/printnode/actions/send-print-job/send-print-job.mjs
(3 hunks)components/printnode/common/constants.mjs
(0 hunks)components/printnode/package.json
(2 hunks)components/printnode/printnode.app.mjs
(1 hunks)components/ragie/actions/create-document/create-document.mjs
(2 hunks)components/ragie/actions/update-document-file/update-document-file.mjs
(2 hunks)components/ragie/common/utils.mjs
(0 hunks)components/ragie/package.json
(2 hunks)components/ragie/ragie.app.mjs
(1 hunks)components/raindrop/actions/parse-bookmark-file/parse-bookmark-file.mjs
(1 hunks)components/raindrop/package.json
(2 hunks)components/ramp/actions/upload-receipt/upload-receipt.mjs
(3 hunks)components/ramp/package.json
(2 hunks)components/reform/actions/extract-data-from-document/extract-data-from-document.mjs
(2 hunks)components/reform/package.json
(2 hunks)components/reform/reform.app.mjs
(1 hunks)components/roboflow/actions/classify-image/classify-image.mjs
(2 hunks)components/roboflow/actions/detect-object-from-image/detect-object-from-image.mjs
(2 hunks)components/roboflow/actions/upload-image/upload-image.mjs
(2 hunks)components/roboflow/common/utils.mjs
(2 hunks)components/roboflow/package.json
(2 hunks)components/roboflow/roboflow.app.mjs
(1 hunks)components/salesforce_rest_api/actions/create-attachment/create-attachment.mjs
(3 hunks)components/salesforce_rest_api/common/sobjects/attachment.mjs
(1 hunks)components/salesforce_rest_api/package.json
(2 hunks)components/scoredetect/actions/create-timestamp-file/create-timestamp-file.mjs
(2 hunks)components/scoredetect/package.json
(2 hunks)components/scoredetect/scoredetect.app.mjs
(2 hunks)components/scrapfly/actions/ai-data-extraction/ai-data-extraction.mjs
(3 hunks)components/scrapfly/package.json
(2 hunks)components/scrapfly/scrapfly.app.mjs
(1 hunks)components/security_reporter/actions/create-finding/create-finding.mjs
(2 hunks)components/security_reporter/actions/update-finding/update-finding.mjs
(2 hunks)components/security_reporter/package.json
(2 hunks)components/security_reporter/security_reporter.app.mjs
(2 hunks)components/sendgrid/actions/send-email-multiple-recipients/send-email-multiple-recipients.mjs
(4 hunks)components/sendgrid/actions/send-email-single-recipient/send-email-single-recipient.mjs
(4 hunks)components/sendgrid/package.json
(2 hunks)components/sftp/actions/upload-file/upload-file.mjs
(2 hunks)components/sftp/package.json
(2 hunks)components/sftp/sftp.app.mjs
(1 hunks)components/signaturit/actions/create-certified-email/create-certified-email.mjs
(2 hunks)components/signaturit/actions/create-signature-request-from-template/create-signature-request-from-template.mjs
(2 hunks)components/signaturit/package.json
(2 hunks)components/signaturit/signaturit.app.mjs
(1 hunks)components/signerx/actions/upload-package/upload-package.mjs
(1 hunks)components/signerx/package.json
(2 hunks)components/signnow/actions/upload-document-with-tags/upload-document-with-tags.mjs
(2 hunks)components/signnow/package.json
(2 hunks)components/smugmug/actions/upload-image/upload-image.mjs
(3 hunks)components/smugmug/package.json
(2 hunks)components/sonix/actions/upload-media/upload-media.mjs
(2 hunks)components/sonix/package.json
(2 hunks)components/tinypng/actions/compress-image/compress-image.mjs
(2 hunks)components/tinypng/package.json
(2 hunks)components/tinypng/tinypng.app.mjs
(1 hunks)components/todoist/actions/import-tasks/import-tasks.mjs
(2 hunks)components/todoist/package.json
(2 hunks)components/todoist/todoist.app.mjs
(1 hunks)components/xero_accounting_api/actions/upload-file/upload-file.mjs
(3 hunks)components/xero_accounting_api/package.json
(2 hunks)components/zerobounce/actions/file-validation/file-validation.mjs
(2 hunks)components/zerobounce/package.json
(2 hunks)components/zip_archive_api/actions/compress-files/compress-files.mjs
(2 hunks)components/zip_archive_api/package.json
(2 hunks)components/zip_archive_api/zip_archive_api.app.mjs
(1 hunks)components/zoho_bugtracker/actions/create-bug/create-bug.mjs
(2 hunks)components/zoho_bugtracker/actions/update-bug/update-bug.mjs
(3 hunks)components/zoho_bugtracker/package.json
(2 hunks)components/zoho_bugtracker/zoho_bugtracker.app.mjs
(1 hunks)components/zoho_catalyst/actions/detect-objects-in-image/detect-objects-in-image.ts
(2 hunks)components/zoho_catalyst/actions/extract-text-from-image/extract-text-from-image.ts
(2 hunks)components/zoho_catalyst/actions/perform-face-detection-and-analysis/perform-face-detection-and-analysis.ts
(3 hunks)components/zoho_catalyst/actions/perform-image-moderation/perform-image-moderation.ts
(2 hunks)components/zoho_catalyst/app/zoho_catalyst.app.ts
(1 hunks)components/zoho_catalyst/common/methods.ts
(1 hunks)components/zoho_catalyst/package.json
(2 hunks)
💤 Files with no reviewable changes (2)
- components/ragie/common/utils.mjs
- components/printnode/common/constants.mjs
🧰 Additional context used
🧬 Code Graph Analysis (4)
components/zoho_catalyst/actions/extract-text-from-image/extract-text-from-image.ts (1)
components/zoho_catalyst/common/methods.ts (1)
getImageFormData
(4-14)
components/zoho_catalyst/actions/perform-image-moderation/perform-image-moderation.ts (1)
components/zoho_catalyst/common/methods.ts (1)
getImageFormData
(4-14)
components/zoho_catalyst/actions/perform-face-detection-and-analysis/perform-face-detection-and-analysis.ts (1)
components/zoho_catalyst/common/methods.ts (1)
getImageFormData
(4-14)
components/zoho_catalyst/actions/detect-objects-in-image/detect-objects-in-image.ts (1)
components/zoho_catalyst/common/methods.ts (1)
getImageFormData
(4-14)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (84)
components/printautopilot/package.json (2)
3-3
: Approve version bump to 0.2.0.
The package version has been correctly incremented to reflect the new release cycle.
16-16
: Verify compatibility with @pipedream/platform ^3.1.0.
Upgrading from ^1.5.1 to ^3.1.0 may include breaking changes in file‐stream and metadata helpers. Please ensure all imports and usages (e.g.,getFileStream
,getFileStreamAndMetadata
) are updated accordingly and run the full test suite to catch any regressions.components/zerobounce/package.json (2)
3-3
: Bump package version to 0.2.0
16-16
: Upgrade@pipedream/platform
dependency to^3.1.0
components/offorte/offorte.app.mjs (1)
11-11
: Add trailing newline for POSIX compliance.
This non-functional update ensures consistent file formatting across components.components/pembee/pembee.app.mjs (1)
11-11
: Add trailing newline for POSIX compliance.
This non-functional update ensures consistent file formatting across components.components/arpoone/arpoone.app.mjs (1)
11-11
: Add trailing newline for POSIX compliance.
This non-functional update ensures consistent file formatting across components.components/postmaster/postmaster.app.mjs (1)
11-11
: Add trailing newline for POSIX compliance.
This non-functional update ensures consistent file formatting across components.components/pixelbin/package.json (2)
3-3
: Confirm CHANGELOG and lockfile regeneration
Version bumped to0.2.0
—please update your CHANGELOG (or release notes) accordingly and regenerate & commit your lockfile (package-lock.json
/yarn.lock
) to ensure reproducible installs.
16-16
: Validate@pipedream/platform@^3.1.0
compatibility
Upgrading to^3.1.0
brings in new file streaming and metadata helpers; double-check that v3.1.x exposes the APIs your refactored components now require and that there are no unintended breaking changes.components/signaturit/package.json (1)
16-17
: Upgrade dependencies to align with file-handling improvements.
Updating@pipedream/platform
to^3.1.0
andform-data
to^4.0.1
aligns this component with the new streaming helpers (and confirms removal of the insecurefs
package).components/smugmug/package.json (2)
3-3
: Bump package version to 0.1.0 for new feature releaseThe minor version increase reflects the addition of file URL/path handling improvements. Please ensure that
CHANGELOG.md
, release notes, and any related documentation are updated to reference this new version.
13-14
: Upgrade platform dependency and drop redundantfs
Moving to
@pipedream/platform@^3.1.0
enables the new file‐stream helpers; removingfs
avoids duplication. Verify that all code now imports the platform utilities (no lingeringfs
imports) and confirm thatform-data@^4.0.0
remains compatible with the updated platform.components/sonix/package.json (1)
17-17
: Confirmform-data
compatibility
Theform-data
version remains at^4.0.0
. Verify there are no transitive conflicts after the platform upgrade, and consider locking this dependency if you encounter version drift.components/scrapfly/package.json (1)
16-16
: Confirm platform ^3.1.0 is compatible with all transitive dependenciesRaising just a heads-up: the caret range
^3.1.0
may pull in3.2.x
once it is released. Please verify downstream code paths (especially new helpers such asgetFileStream
) against the latest@pipedream/platform
API before publishing.components/zoho_catalyst/package.json (2)
3-3
: Version bump recorded – don’t forget the changelog
"version": "0.2.0"
looks correct for the breaking(ish) dependency jump. Be sure to surface the reasons for this bump in the repo-level CHANGELOG / release notes so downstream users can pin appropriately.
17-17
: Large major jump in@pipedream/platform
– verify repo-wide alignmentMoving straight from
^1.5.1
to^3.1.0
can introduce API and TS-type changes. Please grep the monorepo for@pipedream/platform
usage to confirm every package is now ≥3.x; mixing majors in Yarn/PNPM workspaces can silently install two copies.components/zoho_catalyst/actions/extract-text-from-image/extract-text-from-image.ts (1)
9-9
: Action version synced – good
The patch-level increment to0.1.0
matches the new async behaviour.components/zoho_catalyst/actions/detect-objects-in-image/detect-objects-in-image.ts (1)
9-9
: Version bump acknowledged
No issues.components/zoho_catalyst/actions/perform-image-moderation/perform-image-moderation.ts (1)
10-10
: Version bump acknowledgedcomponents/zip_archive_api/package.json (2)
3-3
: Bump the package version following semver conventionsThe component code introduces new props/behaviour (support for URLs, async streaming). A patch-level increment (
0.1.x → 0.2.0
) is fine provided that no breaking change is exposed to existing users. If you removed or renamed any public prop (e.g.file
), consider a MINOR or MAJOR bump instead to signal the behaviour change.
16-16
: Double-check compatibility with@pipedream/platform@^3.1.0
Jumping from
^1.6.5
to^3.1.0
is a major upgrade. Please run the unit / e2e tests for all actions that still rely on legacy helpers (e.g.this.$auth
,this.uploadType
) to confirm that no API surface was removed or renamed.components/zip_archive_api/actions/compress-files/compress-files.mjs (1)
3-5
: Verify helper availability after the platform major upgrade
writeFile
landed in@pipedream/[email protected]
; make sure the runtime deployed to prod/workflows is pinned to ≥ 3.0.0, otherwise this action will fail at load-time.components/reform/package.json (1)
3-17
: Version bump & major-range platform upgrade need a smoke-test
@pipedream/platform
jumps from^1.6.0
to^3.1.0
(major change) while the package version only increments a minor. Double-check for breaking changes in the platform API (e.g. renamed helpers, changed response shapes) across the component before publishing.components/sonix/actions/upload-media/upload-media.mjs (1)
66-74
:knownLength
may beundefined
for remote URLs
form-data
will throw ifknownLength
is falsy but provided. Guard against undefined sizes:-formData.append("file", stream, { - contentType: metadata.contentType, - knownLength: metadata.size, - filename: metadata.name, -}); +formData.append("file", stream, { + contentType: metadata.contentType, + ...(metadata.size ? { knownLength: metadata.size } : {}), + filename: metadata.name, +});components/zoho_bugtracker/package.json (1)
3-17
: Major platform upgrade mirrors Reform componentSame caution as earlier: moving to
^3.1.0
without a major version bump may conceal breaking behaviour. Recommend a1.x
or0.3.0
bump depending on your semver strategy.components/pdffiller/package.json (2)
3-3
: Bump pdffiller component version
Version updated from0.1.1
to0.2.0
.
16-16
: Upgrade@pipedream/platform
dependency
Dependency bumped to^3.1.0
to leverage new file‐streaming utilities. Verify compatibility with your existing code and any peer constraints.components/printnode/printnode.app.mjs (1)
25-25
: SimplifycontentType
description
The updated description correctly removes the outdated file path/URL requirement and points users to the official docs for valid options.components/tinypng/package.json (2)
3-3
: Bump tinypng component version
Version updated from0.1.0
to0.2.0
.
16-16
: Upgrade@pipedream/platform
dependency
Dependency bumped to^3.1.0
for unified file handling across the platform. Ensure no breaking changes for your actions.components/signerx/package.json (2)
3-3
: Bump signerx component version
Version updated from0.1.0
to0.2.0
.
16-16
: Upgrade@pipedream/platform
dependency
Dependency upgraded to^3.1.0
. Confirm compatibility with the refactored upload flow usinggetFileStreamAndMetadata
.components/ragie/package.json (2)
3-3
: Bump ragie component version
Version updated from0.1.0
to0.2.0
.
16-16
: Upgrade@pipedream/platform
dependency
Dependency updated to^3.1.0
. Verify that the actions refactored to usegetFileStreamAndMetadata
operate correctly under this version.components/sftp/package.json (2)
3-3
: Bump version to 0.5.0
Follows semantic versioning and aligns with coordinated updates across components.
13-13
: Upgrade @pipedream/platform to ^3.1.0
Required for the new file streaming and metadata utilities.components/printify/package.json (2)
3-3
: Update version to 0.2.0
Increment aligns with feature rollout and downstream actions refactoring.
16-16
: Upgrade @pipedream/platform to ^3.1.0
Enables standardized file input handling viagetFileStream
helpers.components/todoist/package.json (2)
3-3
: Bump version to 0.2.0
Version bump matches the updated file‐handling enhancements in actions.
13-13
: Upgrade @pipedream/platform to ^3.1.0
Ensures compatibility with the rewritten URL/local path streaming logic.components/salesforce_rest_api/package.json (2)
3-3
: Increment version to 1.6.0
Semantic version increase aligns with new attachment streaming improvements.
13-13
: Upgrade @pipedream/platform to ^3.1.0
Necessary forgetFileStream
and metadata extraction in attachment actions.components/platerecognizer/package.json (2)
3-3
: Bump version to 0.2.0
Reflects the updated file‐recognition action refactor.
16-16
: Upgrade @pipedream/platform to ^3.1.0
Supports the unified streaming approach inrun-recognition
.components/roboflow/package.json (2)
3-3
: Major version bump to 1.0.0
Bumping from 0.x to 1.0.0 signals breaking changes. Ensure you’ve updated the CHANGELOG/release notes and communicated any breaking API alterations to downstream consumers.
16-16
: Upgrade dependency to @pipedream/platform ^3.1.0
This aligns with the new file utilities (getFileStream
, etc.). Double-check that all imported helpers in your action modules match the APIs introduced in v3.1.0.components/meistertask/package.json (2)
3-3
: Minor version bump to 0.1.0
Incrementing the component’s version; confirm that any added functionality (e.g., URL support in attachments) is documented in the release notes.
16-16
: Upgrade dependency to @pipedream/platform ^3.1.0
Required forgetFileStreamAndMetadata
. Verify that the new utility is available and behaves as expected in all action implementations.components/ramp/package.json (2)
3-3
: Minor version bump to 0.2.0
Ensure that consumers are aware of new file-URL support and that any breaking changes (if any) are captured in documentation.
16-16
: Upgrade dependency to @pipedream/platform ^3.1.0
This upgrade is needed for the streaming refactor inupload-receipt
. Confirm all file streaming helpers are correctly imported and tested.components/sendgrid/package.json (2)
3-3
: Minor version bump to 0.5.0
Please update the CHANGELOG with details on attachment URL support and the switch to streaming APIs.
13-13
: Upgrade dependency to @pipedream/platform ^3.1.0
This ensures availability ofgetFileStream
. Verify that both single‐ and multi‐recipient actions import the correct helper and pass metadata properly.components/signnow/package.json (2)
3-3
: Minor version bump to 0.2.0
Document the removal of thefs
package and the adoption of platform file utilities in your release notes.
16-16
: Upgrade dependency to @pipedream/platform ^3.1.0
Required forgetFileStreamAndMetadata
in file upload actions. Confirm that this version covers all platform changes your code relies on.components/xero_accounting_api/package.json (2)
3-3
: Patch vs minor: consider semver signallingJumping from
0.2.1
→0.3.0
communicates a breaking change. If the only delta is dependency bumps,0.2.2
(patch) or0.2.3
(patch) may be clearer unless the new@pipedream/platform
v3 API is required at runtime.Please confirm that consumer-facing behaviour changed; otherwise downgrade to a patch bump.
13-13
:@pipedream/platform
v3 is a breaking upgrade—verify runtime code importsActions that previously imported helpers from
@pipedream/platform
v1 may have moved/renamed symbols. Ensure all.mjs
files under this package build and import correctly.#!/bin/bash # Quick grep to list platform imports for spot-checking fd -e mjs | xargs rg --line-number '@pipedream/platform' -ncomponents/scoredetect/package.json (1)
16-18
: Unusedform-data
suspicionOnly keep
form-data
if one of the component files explicitly imports it.Run:
#!/bin/bash rg --count 'from .*form-data' || echo "no direct import found"components/security_reporter/package.json (1)
3-3
: Major bump from 0.1 → 0.2 matches platform upgrade—LGTMcomponents/raindrop/package.json (1)
20-22
: ```shell
#!/bin/bashComprehensive search for all references to “got”
rg -nE "require(['"]got['"])"
rg -nE "import\s+.*\s+got"
rg -nE "from\s+['"]got['"]"</details> <details> <summary>components/sftp/actions/upload-file/upload-file.mjs (2)</summary> `2-4`: **Dependency switch looks solid – but verify minimum platform version.** `getFileStream` was introduced in `@pipedream/[email protected]`. Make sure the package.json for this component (and the monorepo root) pins `^3.1.0` or higher, otherwise users on an older lock-file will get runtime failures. --- `34-34`: **Great UX improvement on the validation message.** </details> <details> <summary>components/todoist/actions/import-tasks/import-tasks.mjs (1)</summary> `3-3`: **👍 Switched to `getFileStream` – drop the heavy `fs` dependency.** </details> <details> <summary>components/tinypng/tinypng.app.mjs (1)</summary> `10-12`: **Nice consolidation of path & URL into one prop.** Reduces cognitive load and mirrors the rest of the PR. </details> <details> <summary>components/scoredetect/actions/create-timestamp-file/create-timestamp-file.mjs (1)</summary> `20-27`: **LGTM – unified file handling is clean and concise** The switch to `getFileStreamAndMetadata` removes the previous URL/file-type branching and improves readability. </details> <details> <summary>components/reform/reform.app.mjs (1)</summary> `9-11`: **Prop description update looks good** Clearer wording eliminates the former “local-only” assumption and aligns with the new helper. </details> <details> <summary>components/zoho_bugtracker/actions/update-bug/update-bug.mjs (1)</summary> `5-5`: **Import looks good** Switching to the platform helper keeps the codebase consistent with the new standard. </details> <details> <summary>components/reform/actions/extract-data-from-document/extract-data-from-document.mjs (1)</summary> `4-4`: **Good move to platform helper** Dependency removal of `fs` keeps the bundle lighter. </details> <details> <summary>components/ragie/ragie.app.mjs (1)</summary> `10-12`: **👍 Clearer UX copy** Explicitly spelling out that either a URL or `/tmp` path works will cut down support requests. </details> <details> <summary>components/security_reporter/security_reporter.app.mjs (1)</summary> `3-5`: **Import clean-up acknowledged** Good to see redundant helpers removed. </details> <details> <summary>components/pdffiller/pdffiller.app.mjs (1)</summary> `47-49`: **Label/description update looks good** The tweak accurately reflects the new dual-input behaviour adopted across the repo. </details> <details> <summary>components/sendgrid/actions/send-email-single-recipient/send-email-single-recipient.mjs (1)</summary> `181-183`: **Clearer prop wording – nice improvement** Accepting both URLs and `/tmp` paths removes friction for users. </details> <details> <summary>components/sendgrid/actions/send-email-multiple-recipients/send-email-multiple-recipients.mjs (1)</summary> `160-162`: **Prop label/description updated – 👍** Matches the new URL+/tmp convention. </details> <details> <summary>components/security_reporter/actions/update-finding/update-finding.mjs (1)</summary> `148-151`: **Minor UX polish – good** The clarified label and description make the prop self-explanatory. </details> <details> <summary>components/salesforce_rest_api/common/sobjects/attachment.mjs (1)</summary> `12-14`: **Prop description updated – looks good** Consistent with new cross-component file conventions. </details> <details> <summary>components/ramp/actions/upload-receipt/upload-receipt.mjs (2)</summary> `3-3`: **Unused import removed but new helper added – good!** Switching from `fs.readFileSync` to `getFileStream` removes the sync I/O and temp-file guard. Nice move. --- `27-29`: **Prop description updated – OK but watch consistency** Other components migrated the prop name itself (`file` / `filePathOrUrl`). Leaving it as `filePath` is fine for backward compatibility, just ensure docs and recipes reference the new semantics. </details> <details> <summary>components/smugmug/actions/upload-image/upload-image.mjs (3)</summary> `2-3`: **👍 Switched to `getFileStreamAndMetadata`** Async stream + metadata is cleaner than manual `fs` + stat calls. --- `22-24`: **Prop wording aligned – good** Label/description consistently mirrors other components. --- `35-42`: **Gracefully handle missing metadata** `metadata.contentType` and `metadata.size` can be `undefined` when the remote server omits headers. `form-data` will throw if `knownLength` is `undefined` or negative. ```diff - contentType: metadata.contentType, - knownLength: metadata.size, + contentType: metadata.contentType || "application/octet-stream", + ...(Number.isFinite(metadata.size) && metadata.size > 0 + ? { knownLength: metadata.size } + : {}),
Prevents runtime failures with edge-case URLs.
components/signaturit/signaturit.app.mjs (1)
35-37
: Metadata text updated – looks good
No functional impact; change aligns wording with new file-handling capability.components/tinypng/actions/compress-image/compress-image.mjs (1)
1-1
: Helper import modernises file handling
Good replacement for manualfs
logic.components/roboflow/actions/detect-object-from-image/detect-object-from-image.mjs (1)
35-42
: Content-Type header may be incorrect for a raw base64 payload
utils.getBase64File()
returns a base-64 string, yet the request is sent with
"Content-Type": "application/x-www-form-urlencoded"
.
UnlessdetectObject
re-encodes the body internally, this MIME type will mislead the server and break signature verification on some gateways.Double-check the Roboflow API docs; if the endpoint expects a JSON body like
{ image: "<base64>" }
, setapplication/json
.
If it expects plain text, usetext/plain
.
Otherwise, remove the header and let the SDK pick the correct one.- headers: { - "Content-Type": "application/x-www-form-urlencoded", - }, + // let the SDK set the header, or explicitly: + headers: { "Content-Type": "application/json" },components/roboflow/actions/classify-image/classify-image.mjs (1)
34-42
: Verify request body format matches Roboflow APIRoboflow’s classification endpoint expects URL-encoded data with a
base64
orfile
field (per docs). Ensureutils.getBase64File()
returns a payload such asbase64=<data>
rather than raw binary, otherwise the API will respond with 400.
components/zip_archive_api/actions/compress-files/compress-files.mjs
Outdated
Show resolved
Hide resolved
components/signnow/actions/upload-document-with-tags/upload-document-with-tags.mjs
Show resolved
Hide resolved
components/xero_accounting_api/actions/upload-file/upload-file.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.
Hi @GTFalcao overall lgtm! I've added just a few suggestions here and there. Reeady for QA in the meantime!
}; |
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.
can you omit this kind of changes?
}; | |
}; |
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.
I always hide whitespace changes when reviewing the PR, this was likely done by ESLint when merging with master and think it's easier to merge it since it will keep happening to any future branches until it's merged
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.
I guess you need to increase the version for the rest of components
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.
I've been bumping only the versions of affected components since we're already publishing several dozen at once. We'd be publishing hundreds of components if we increased all the versions - dunno if that might present a problem when publishing
|
||
export default { | ||
key: "meistertask-create-attachment", | ||
name: "Create Attachment", | ||
description: "Create a new attachment. [See the docs](https://developers.meistertask.com/reference/post-attachment)", | ||
version: "0.0.1", | ||
version: "0.1.0", |
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.
I thought we were changing to major version
version: "0.1.0", | |
version: "1.0.0", |
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.
Only if a prop name is changed or props are added/removed, as that requires user action after updating. If the props are kept the same, users can update and their input will remain functional (if the prop takes a path to /tmp and/or a URL)
@@ -146,8 +146,8 @@ export default { | |||
}, | |||
draftDocumentsFile: { | |||
type: "string[]", | |||
label: "Draft Documents File ", | |||
description: "The path to a file in the `/tmp` directory. [See the documentation on working with files](https://pipedream.com/docs/code/nodejs/working-with-files/#writing-a-file-to-tmp).", | |||
label: "Draft Documents File", |
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.
label: "Draft Documents File", | |
label: "Draft Document File Paths Or Urls", |
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.
Thanks! I'll double check these as I might have missed more
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
♻️ Duplicate comments (1)
components/tinypng/actions/compress-image/compress-image.mjs (1)
20-32
: Memory inefficiency: Avoid buffering entire image in RAM.This implementation still buffers the entire image into memory, which was flagged in previous reviews. TinyPNG's API supports streaming uploads, so you should pass the stream directly instead of reading all chunks into a buffer.
Also, the header key should be capitalized for consistency.
Apply this diff to stream directly and fix header capitalization:
- const { - stream, metadata, - } = await getFileStreamAndMetadata(this.file); - - const chunks = []; - for await (const chunk of stream) { - chunks.push(chunk); - } - const data = Buffer.concat(chunks); - const headers = { - "Content-Type": metadata.contentType, - "content-disposition": "attachment; filename=" + metadata.name, - }; + const { stream, metadata } = await getFileStreamAndMetadata(this.file); + const data = stream; // pass stream directly + const headers = { + "Content-Type": metadata.contentType, + "Content-Disposition": "attachment; filename=" + metadata.name, + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/printnode/actions/send-print-job/send-print-job.mjs
(3 hunks)components/raindrop/actions/parse-bookmark-file/parse-bookmark-file.mjs
(1 hunks)components/roboflow/actions/upload-image/upload-image.mjs
(2 hunks)components/security_reporter/actions/create-finding/create-finding.mjs
(2 hunks)components/security_reporter/actions/update-finding/update-finding.mjs
(2 hunks)components/tinypng/actions/compress-image/compress-image.mjs
(2 hunks)components/zip_archive_api/actions/compress-files/compress-files.mjs
(2 hunks)components/zoho_catalyst/common/methods.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- components/zoho_catalyst/common/methods.ts
- components/roboflow/actions/upload-image/upload-image.mjs
- components/zip_archive_api/actions/compress-files/compress-files.mjs
- components/security_reporter/actions/create-finding/create-finding.mjs
- components/security_reporter/actions/update-finding/update-finding.mjs
- components/raindrop/actions/parse-bookmark-file/parse-bookmark-file.mjs
🔇 Additional comments (7)
components/tinypng/actions/compress-image/compress-image.mjs (3)
1-1
: LGTM - Platform utility import added correctly.The import aligns with the standardization effort to use platform utilities for file handling.
8-8
: Appropriate version bump for major refactoring.The major version increment correctly reflects the significant changes in file handling approach.
34-42
: API call and response handling look correct.The compression API call and response handling follow the expected pattern.
components/printnode/actions/send-print-job/send-print-job.mjs (4)
2-2
: LGTM: Import statement correctly adds file streaming utility.The import of
getFileStream
from@pipedream/platform
aligns with the standardization effort described in the PR objectives.
8-8
: LGTM: Appropriate version bump for breaking changes.The major version increment from "0.0.1" to "1.0.0" correctly reflects the breaking change from separate
filePath
/fileUrl
properties to a single consolidated property.
26-27
: LGTM: Property consolidation improves usability.The consolidation of separate file path and URL properties into a single "File Path or URL" property simplifies the interface while maintaining flexibility to accept both local paths and URLs.
69-69
: Verify removal of fileUrl property reference.The destructuring no longer includes
fileUrl
, which aligns with the property consolidation. Ensure all references to the oldfileUrl
property have been removed from the codebase.#!/bin/bash # Description: Verify that fileUrl property references have been removed from printnode components # Expected: No remaining references to fileUrl in printnode component files rg -A 3 -B 3 "fileUrl" components/printnode/
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 |
About
Also @vunguyenhung , there are some components in this PR that were not included in the test report:
|
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 |
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
🧹 Nitpick comments (1)
components/todoist/actions/export-tasks/export-tasks.mjs (1)
1-42
: Consider adopting the standardized file handling approach.This component still uses direct
fs
operations andtmp-promise
, which appears inconsistent with the PR's objective of standardizing file handling using@pipedream/platform
helpers likegetFileStream
andgetFileStreamAndMetadata
. Consider refactoring to align with the overall modernization effort.For consistency with the PR's file handling standardization, consider updating this component to use
@pipedream/platform
helpers:-import fs from "fs"; -import { file } from "tmp-promise"; +import { getFileStream } from "@pipedream/platform";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
components/todoist/actions/export-tasks/export-tasks.mjs
(2 hunks)components/todoist/actions/import-tasks/import-tasks.mjs
(2 hunks)components/todoist/package.json
(2 hunks)components/zoho_bugtracker/actions/create-bug/create-bug.mjs
(2 hunks)components/zoho_bugtracker/actions/update-bug/update-bug.mjs
(3 hunks)components/zoho_bugtracker/common/utils.mjs
(0 hunks)
💤 Files with no reviewable changes (1)
- components/zoho_bugtracker/common/utils.mjs
🚧 Files skipped from review as they are similar to previous changes (4)
- components/todoist/actions/import-tasks/import-tasks.mjs
- components/zoho_bugtracker/actions/update-bug/update-bug.mjs
- components/zoho_bugtracker/actions/create-bug/create-bug.mjs
- components/todoist/package.json
🔇 Additional comments (2)
components/todoist/actions/export-tasks/export-tasks.mjs (2)
10-10
: Version bump is appropriate for the functional change.The minor version increment from "0.0.3" to "0.1.0" is appropriate given the change from asynchronous to synchronous CSV conversion.
30-30
: ```shell
#!/bin/bashVerify json-2-csv dependency version and how it’s imported/used in the repo
1. Check if json-2-csv is listed in package.json and its version
grep -R ""json-2-csv"" -n package.json || echo "json-2-csv not found in package.json"
echo
2. Locate import or require statements for json-2-csv
rg "import .*json-2-csv" -n
rg "require('json-2-csv')" -n .echo
3. Search for any references to the async API
rg "json2csvAsync" -n .
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
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 |
Partially addresses #16977
Summary by CodeRabbit
New Features
/tmp
paths across many integrations, enhancing user flexibility.Bug Fixes
Documentation
Refactor
Chores