Skip to content

fix(firestore-bigquery-export): use correct Firestore database ID in document_name when exporting to BigQuery (rebased)#2781

Open
IzaakGough wants to merge 5 commits intonextfrom
rebased-dbidfix
Open

fix(firestore-bigquery-export): use correct Firestore database ID in document_name when exporting to BigQuery (rebased)#2781
IzaakGough wants to merge 5 commits intonextfrom
rebased-dbidfix

Conversation

@IzaakGough
Copy link
Copy Markdown
Contributor

@IzaakGough IzaakGough commented Apr 20, 2026

Fixes #2563

supersedes #2580

Copy link
Copy Markdown
Contributor

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

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 enables the use of non-default Firestore database IDs in the BigQuery export script by introducing a firestoreInstanceId configuration. It also removes the onStartResize event logic from the storage-resize-images extension. Feedback points out an inconsistency between the code and extension.yaml regarding the onStart event, suggests using the new FIRESTORE_DEFAULT_DATABASE constant more broadly for consistency, and notes that the removal of the resize event is a breaking change that should likely be handled in a separate PR.

I am having trouble creating individual review comments. Click here to see my feedback.

storage-resize-images/functions/src/index.ts (152)

high

The call to events.recordStartEvent(object) has been removed, but the onStart event remains defined in extension.yaml (line 410). This results in the event never being triggered, which is an inconsistency in the extension's behavior. If the event is no longer needed, it should be removed from extension.yaml. Otherwise, the call should be restored.

    await events.recordStartEvent(object);
    await generateResizedImageHandler(object);

firestore-bigquery-export/scripts/import/src/config.ts (7)

medium

The constant FIRESTORE_DEFAULT_DATABASE is defined here but is not used to replace the literal string "(default)" in the rest of the file (e.g., lines 283 and 328). To improve maintainability and ensure consistency, please use this constant throughout the configuration logic.

storage-resize-images/extension.yaml (430-434)

medium

Removing the onStartResize event is a breaking change for users who may be relying on this event for integration. Furthermore, these changes to the storage-resize-images extension seem unrelated to the PR's primary goal of fixing Firestore database IDs in the BigQuery export script. Consider moving these changes to a separate PR and documenting the breaking change in the release notes.

@cabljac cabljac changed the title Rebased dbidfix fix(firestore-bigquery-export): use correct Firestore database ID in document_name when exporting to BigQuery (rebased) Apr 20, 2026
@cabljac cabljac marked this pull request as ready for review April 21, 2026 15:41
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.

🐛 [fs-bq-import-collection] replaces Firestore database ID with (default) in document_name when importing to BigQuery

3 participants