Google Cloud BigQuery bug fix #18647
Conversation
| The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughBumps many Google Cloud component versions; BigQuery new-row source switches to BigQuery client query execution and removes its deactivate hook; common BigQuery run() becomes async; Google Cloud app drops Compute VM helper methods; several actions bump metadata versions. Changes
Sequence Diagram(s)sequenceDiagram autonumber actor Runner as Source Runner participant SRC as bigquery-new-row source participant BQ as BigQuery Client participant JOB as Query Job Runner->>SRC: run(event) / poll() activate SRC SRC->>BQ: createQueryJob({ query, params }) BQ-->>SRC: job SRC->>JOB: await job.getQueryResults() / job.promise() JOB-->>SRC: rows (paged) loop per page SRC->>SRC: process rows, dedupe/emit alt continue paging SRC->>JOB: getQueryResults(nextPageToken) end end SRC->>SRC: update lastResultId deactivate SRC Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/google_cloud/package.json(1 hunks)components/google_cloud/sources/bigquery-new-row/bigquery-new-row.mjs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
components/google_cloud/sources/bigquery-new-row/bigquery-new-row.mjs Outdated Show resolved Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/google_cloud/sources/bigquery-new-row/bigquery-new-row.mjs (1)
102-106: Qualify the table with dataset to avoid resolution issues.These queries reference the table without a dataset. Either fully qualify it or set defaultDataset on the job.
- FROM \`${this.tableId}\` + FROM \`${this.datasetId}.${this.tableId}\`
♻️ Duplicate comments (1)
components/google_cloud/sources/bigquery-new-row/bigquery-new-row.mjs (1)
140-152: Honor falsy-but-valid lastResultId (0, "").Using if (lastResultId) skips these legitimate values, causing re-scan and duplicates. Compare against null/undefined instead. This mirrors prior feedback.
- if (lastResultId) { + if (lastResultId !== null && lastResultId !== undefined) { query += ` WHERE \`${this.uniqueKey}\` >= @lastResultId`; } @@ - ...(lastResultId - ? { - lastResultId, - } - : {}), + ...((lastResultId !== null && lastResultId !== undefined) + ? { lastResultId } + : {}),
🧹 Nitpick comments (2)
components/google_cloud/sources/common/bigquery.mjs (2)
138-144: Clearing rows: prefer reassignment or length=0; splice isn’t faster.rows is local and re-assigned each page; explicit clearing is unnecessary. If you keep it, rows.length = 0 is simpler and typically fastest.
- rows.splice(0, rows.length); // More efficient than rows.length = 0 + // rows.length = 0; // simple and fast, or omit clearing entirely
182-186: Nit: avoid redundant return await.return await in async functions adds no benefit without try/catch.
- return await this.processCollection(queryOpts, timestamp); + return this.processCollection(queryOpts, timestamp);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/google_cloud/sources/bigquery-new-row/bigquery-new-row.mjs(3 hunks)components/google_cloud/sources/bigquery-query-results/bigquery-query-results.mjs(1 hunks)components/google_cloud/sources/common/bigquery.mjs(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/google_cloud/sources/bigquery-query-results/bigquery-query-results.mjs
🧰 Additional context used
🧬 Code graph analysis (2)
components/google_cloud/sources/bigquery-new-row/bigquery-new-row.mjs (1)
components/google_cloud/sources/common/bigquery.mjs (5)
client(54-56)job(64-66)queryOpts(184-184)rows(92-95)maxRowsPerExecution(73-73)
components/google_cloud/sources/common/bigquery.mjs (1)
components/google_cloud/sources/bigquery-new-row/bigquery-new-row.mjs (5)
queryOpts(107-112)job(116-118)client(113-115)maxRowsPerExecution(134-134)rows(119-121)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
components/google_cloud/sources/common/bigquery.mjs (1)
29-33: Max rows per execution defaults look good.Lower default (1000) and raising max (10000) are reasonable safeguards for memory.
components/google_cloud/sources/bigquery-new-row/bigquery-new-row.mjs (2)
12-12: Version bump OK.
113-121: Use BigQuery client for createQueryJob instead of Dataset.createQueryJob
- Dataset.createQueryJob returns an object without getQueryResults; replace with:
- const client = this.googleCloud.getBigQueryClient().dataset(this.datasetId); - const [job] = await client.createQueryJob(queryOpts); + const bigquery = this.googleCloud.getBigQueryClient(); + const opts = { ...queryOpts, defaultDataset: { datasetId: this.datasetId } }; + const [job] = await bigquery.createQueryJob(opts);
- Optional: narrow
SELECT *toSELECT \${this.uniqueKey}`` to reduce payload.Likely an incorrect or invalid review comment.
components/google_cloud/sources/bigquery-new-row/bigquery-new-row.mjs Outdated Show resolved Hide resolved
components/google_cloud/sources/bigquery-new-row/bigquery-new-row.mjs Outdated Show resolved Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/google_cloud/actions/switch-instance-boot-status/switch-instance-boot-status.mjs (1)
77-90: Consider rate limiting in the wait loop.The
whileloop continuously pollsoperationsClient.wait()without an explicit delay between iterations. If the Google Cloud SDK'swait()method doesn't include built-in rate limiting, this could result in excessive API calls for long-running operations.Consider adding a small delay between iterations or verify that
operationsClient.wait()includes built-in throttling:async waitOperation(operation) { const operationsClient = this.zoneOperationsClient(); const sdkParams = this.googleCloud.sdkParams(); while (operation.status !== "DONE") { [ operation, ] = await operationsClient.wait({ operation: operation.name, project: sdkParams.projectId, zone: operation.zone.split("/").pop(), }); + // Add a small delay to prevent excessive API calls if wait() doesn't throttle + if (operation.status !== "DONE") { + await new Promise(resolve => setTimeout(resolve, 1000)); + } } return operation; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
components/google_cloud/actions/bigquery-insert-rows/bigquery-insert-rows.mjs(1 hunks)components/google_cloud/actions/create-bucket/create-bucket.mjs(1 hunks)components/google_cloud/actions/create-scheduled-query/create-scheduled-query.mjs(1 hunks)components/google_cloud/actions/get-bucket/get-bucket.mjs(1 hunks)components/google_cloud/actions/get-object/get-object.mjs(1 hunks)components/google_cloud/actions/list-buckets/list-buckets.mjs(1 hunks)components/google_cloud/actions/logging-write-log/logging-write-log.mjs(1 hunks)components/google_cloud/actions/run-query/run-query.mjs(1 hunks)components/google_cloud/actions/search-objects/search-objects.mjs(1 hunks)components/google_cloud/actions/switch-instance-boot-status/switch-instance-boot-status.mjs(4 hunks)components/google_cloud/actions/upload-object/upload-object.mjs(1 hunks)components/google_cloud/google_cloud.app.mjs(0 hunks)components/google_cloud/sources/bigquery-query-results/bigquery-query-results.mjs(1 hunks)components/google_cloud/sources/new-pubsub-messages/new-pubsub-messages.mjs(1 hunks)
💤 Files with no reviewable changes (1)
- components/google_cloud/google_cloud.app.mjs
✅ Files skipped from review due to trivial changes (9)
- components/google_cloud/actions/bigquery-insert-rows/bigquery-insert-rows.mjs
- components/google_cloud/actions/list-buckets/list-buckets.mjs
- components/google_cloud/actions/run-query/run-query.mjs
- components/google_cloud/sources/new-pubsub-messages/new-pubsub-messages.mjs
- components/google_cloud/actions/create-bucket/create-bucket.mjs
- components/google_cloud/actions/get-object/get-object.mjs
- components/google_cloud/actions/logging-write-log/logging-write-log.mjs
- components/google_cloud/actions/upload-object/upload-object.mjs
- components/google_cloud/actions/get-bucket/get-bucket.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- components/google_cloud/sources/bigquery-query-results/bigquery-query-results.mjs
🧰 Additional context used
🧬 Code graph analysis (1)
components/google_cloud/actions/switch-instance-boot-status/switch-instance-boot-status.mjs (1)
components/google_cloud/google_cloud.app.mjs (2)
zones(16-16)instances(26-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (8)
components/google_cloud/actions/search-objects/search-objects.mjs (1)
5-5: LGTM!The version bump from "0.0.4" to "0.0.5" is appropriate as part of the broader package update.
components/google_cloud/actions/create-scheduled-query/create-scheduled-query.mjs (1)
15-15: LGTM! Metadata version bump aligns with package update.The version increment from "0.0.2" to "0.0.3" is appropriate as part of the broader Google Cloud package update (0.6.2 → 0.6.3 as noted in the PR summary). No functional changes were made to this action.
components/google_cloud/actions/switch-instance-boot-status/switch-instance-boot-status.mjs (6)
2-7: LGTM!The imports are correctly structured and all are utilized in the methods section below.
11-11: LGTM!Version increment is appropriate for the refactoring changes.
22-40: LGTM!The props are well-structured with appropriate UI metadata and dynamic options. The zone dependency in
instanceNameis correctly handled with the guard clause on Line 36.
57-76: LGTM!Client factory methods and
listZones()are correctly implemented using the Google Cloud Compute SDK.
91-119: LGTM!Both
listVmInstancesByZone()andswitchInstanceBootStatus()are correctly implemented. The validation and dynamic method invocation inswitchInstanceBootStatus()are handled properly.
121-141: LGTM!The
run()method correctly orchestrates the boot status switch operation with optional wait for completion. The logic flow is clear and appropriate.
Resolves #18486
Summary by CodeRabbit