Skip to content

feat(firestore-bigquery-export): add the sync queue params - #3129

Open
cabljac wants to merge 1 commit into
feat/kits-fbe-sync-task-modulefrom
feat/kits-fbe-sync-queue-params
Open

feat(firestore-bigquery-export): add the sync queue params#3129
cabljac wants to merge 1 commit into
feat/kits-fbe-sync-task-modulefrom
feat/kits-fbe-sync-queue-params

Conversation

@cabljac

@cabljac cabljac commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Third layer of the Cloud Tasks write buffer stack. Adds MAX_DISPATCHES_PER_SECOND and MAX_ENQUEUE_ATTEMPTS with the extension's defaults of 100 and 3 and its validation ranges, exposes the first as a config expression for the queue's rate limit, and adds the matching optional ExportConfig fields with defaults applied in resolveExportConfig. Nothing reads the new fields yet.

One subtlety worth a look: IntParam.value() is parseInt(env || "0", 10) || 0 and never consults the declared default, so an unset param would reach resolveExportConfig as 0 and defeat the ?? 100 and ?? 3 fallbacks. optionalInt reports unset and blank env vars as undefined and passes an explicit 0 through as a real setting. The test mock for defineInt mirrors the real behaviour so the tests would catch a regression.

81 tests pass.

Part of #3031.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces two new configuration parameters to the Firestore BigQuery Export extension: MAX_DISPATCHES_PER_SECOND and MAX_ENQUEUE_ATTEMPTS, along with corresponding tests and helper functions to handle their defaults. The review comments correctly point out that the validation regexes for both parameters do not allow empty strings, which would cause deployment-time validation failures if left blank. Modifying these regexes to allow empty values is recommended.

Comment thread kits/firestore-bigquery-export/src/config.ts Outdated
Comment thread kits/firestore-bigquery-export/src/config.ts Outdated
@cabljac
cabljac force-pushed the feat/kits-fbe-sync-queue-params branch from 713ed1d to bcc01ef Compare September 7, 2026 16:45
@cabljac
cabljac marked this pull request as ready for review September 7, 2026 16:50
@cabljac
cabljac force-pushed the feat/kits-fbe-sync-queue-params branch from bcc01ef to b3438b6 Compare September 7, 2026 16:52
@cabljac cabljac mentioned this pull request Sep 7, 2026
62 tasks
Adds MAX_DISPATCHES_PER_SECOND and MAX_ENQUEUE_ATTEMPTS with the
extension's defaults of 100 and 3 and its validation ranges, plus the
matching ExportConfig fields. IntParam.value() never consults the declared
default, so unset and blank values are reported as undefined and the
default applies in resolveExportConfig; an explicit 0 is preserved.

Nothing reads the new fields yet.
@cabljac
cabljac force-pushed the feat/kits-fbe-sync-queue-params branch from b3438b6 to d9f7bd9 Compare September 7, 2026 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant