fix(firestore-bigquery-export): use correct Firestore database ID in document_name when exporting to BigQuery (rebased)#2781
fix(firestore-bigquery-export): use correct Firestore database ID in document_name when exporting to BigQuery (rebased)#2781IzaakGough wants to merge 5 commits intonextfrom
Conversation
…led directory" This reverts commit f505540.
There was a problem hiding this comment.
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)
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)
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)
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.
Fixes #2563
supersedes #2580