[go: nahoru, domu]

Skip to content
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

feat(firestore-bigquery-export): allow firestore database selection #1933

Merged
merged 7 commits into from
Feb 28, 2024

Conversation

jauntybrain
Copy link
Contributor
@jauntybrain jauntybrain commented Jan 26, 2024

Adds support for the multi-database feature recently introduced in Firestore.
Allows users to specify a database ID for their Firestore instance.

Includes non-breaking dependency and code changes for firestore-bigquery-change-tracker.

Resolves #1743.

@jauntybrain jauntybrain requested a review from a team as a code owner January 26, 2024 04:47
@pr-Mais
Copy link
Member
pr-Mais commented Jan 26, 2024

@jauntybrain conflict

@pr-Mais
Copy link
Member
pr-Mais commented Jan 31, 2024

can you add this new param to the _emulator's env file, that might be the reason tests are failing

@amungi
Copy link
amungi commented Feb 6, 2024

Hi @jauntybrain / @pr-Mais : Can this feature for allowing firestore database selection be made available in the February release of the firestore-bigquery-export extension ? It would be really helpful for some of our projects where we are using multiple firestore databases in the same firebase project.

@amungi
Copy link
amungi commented Feb 13, 2024

Hi @jauntybrain / @pr-Mais - is there any way to speed up the release of this feature?

@IchordeDionysos
Copy link
Contributor

@pr-Mais any chance we could get this merged and uploaded to https://extensions.dev? ☺️
This is blocking us right now to setup some important analytics on data we added to a secondary database.

Given that support for multiple databases is now GA, this would be really helpful to have!

@pr-Mais
Copy link
Member
pr-Mais commented Feb 28, 2024

Thanks all for your patience, we will release it ASAP.

@pr-Mais pr-Mais merged commit 15cfa37 into next Feb 28, 2024
7 checks passed
@pr-Mais pr-Mais deleted the @invertase/support-multiple-databases branch February 28, 2024 13:55
@jauntybrain
Copy link
Contributor Author

Hey all! Unfortunately, we have to pause the release of this feature for now. Extensions currently do not support v2 triggers that are required for named database selection (v1 trigger SDK does not support this feature).

We will be monitoring Firebase updates and implement this feature as soon as the v2 function support is available.

@IchordeDionysos
Copy link
Contributor
IchordeDionysos commented Feb 28, 2024

Then why does the Firestore SDK support setting the database for Cloud Functions v1? 🤔

Okay that option was added 6 years ago...

@amungi
Copy link
amungi commented Feb 29, 2024

@jauntybrain - some extensions, such as the Search Firestore with Algolia ( https://extensions.dev/extensions/algolia/firestore-algolia-search ) allow the Database ID to be configured, and we can specify Firestore database IDs other than the (default) database. How is it that some extensions have been able to do this without running into the v2 trigger limitations ?

@IchordeDionysos
Copy link
Contributor

@jauntybrain well it seems like firestore-algolia-search does not properly support multiple databases.
the databaseId is only taken into account for the reindexing operation but not the real-time updates:

https://github.com/search?q=repo%3Aalgolia%2Ffirestore-algolia-search%20databaseId&type=code
https://github.com/algolia/firestore-algolia-search/blob/8976e65d4be4878b3ffcb9709a9725b38696da87/functions/src/index.ts#L44

For the real-time index operation the database is not configured:
https://github.com/algolia/firestore-algolia-search/blob/8976e65d4be4878b3ffcb9709a9725b38696da87/functions/src/index.ts#L134-L136

@IchordeDionysos
Copy link
Contributor

@jauntybrain @pr-Mais couldn't the extension use EventArc triggers and use Firestore as an event source? 🤔
https://firebase.google.com/docs/extensions/publishers/functions#customevents

This would work with Cloud Functions V2 and seems to allow using non-default databases:
https://cloud.google.com/firestore/docs/extend-with-functions-2nd-gen#functions_cloudevent_firebase_firestore-nodejs

@jauntybrain
Copy link
Contributor Author

Hi @IchordeDionysos, thank you for this suggestion. I'll review this approach to see if Eventarc triggers can be a suitable replacement for Firestore triggers until v2 support is added.

@jauntybrain
Copy link
Contributor Author

Hi everyone. After extensive testing, it seems like this feature is working well after all. The Firestore trigger correctly recognizes doc changes from a secondary database, so no EventArc implementation is needed.

We will be releasing this feature in v0.1.46, as originally intended.

@amungi
Copy link
amungi commented Mar 5, 2024

@jauntybrain - Thats great news !

@IchordeDionysos
Copy link
Contributor

@jauntybrain any rough idea when you'd be able to push the release? ☺️

@vmalyi
Copy link
vmalyi commented Mar 19, 2024

@jauntybrain, I'm curious what's the timeline for releasing the update containing this.

Thanks!

pr-Mais added a commit that referenced this pull request Mar 20, 2024
…1933)

Co-authored-by: Mais Alheraki <mais.alheraki@gmail.com>
@alexregier
Copy link

Hello @jauntybrain , we just tested the update but can't get it working. When installing the extension we get an error
Cannot create Firestore trigger for projects/XYZ/databases/ABC. Triggers must be created with a \"(default)\" Firestore Native database. See https://firebase.google.com/docs/firestore/extend-with-functions#limitations..

So it does not seem to work with v1 functions and that's also what's mentioned in the docs.

@jauntybrain
Copy link
Contributor Author

Hi @alexregier, thanks for reporting this. Looking into it now.

@jauntybrain
Copy link
Contributor Author

@alexregier I just tested version 0.1.48 and was able to successfully listen to changes in a secondary database instance with no errors. Could you please provide the full configuration of the extension with any sensitive information redacted?

@alexregier
Copy link

@jauntybrain I configured the extension via terraform, but this is the configuration to be used:

extension_ref                 = "firebase/firestore-bigquery-export"
extension_version             = "0.1.46"
params                        = {
   "BIGQUERY_PROJECT_ID"           = "XYZ"
   "COLLECTION_PATH"               = "someCollection/{someDoc}/anotherCollection"
   "DATABASE_ID"                   = "ABC"
   "DATASET_ID"                    = "abc"
   "DATASET_LOCATION"              = "eu"
   "DO_BACKFILL"                   = "yes"
   "TABLE_ID"                      = "abc"
   "TABLE_PARTITIONING"            = "MONTH"
   "TIME_PARTITIONING_FIELD"       = "timestamp"
   "TIME_PARTITIONING_FIELD_TYPE"  = "TIMESTAMP"
   "USE_NEW_SNAPSHOT_QUERY_SYNTAX" = "yes"
   "WILDCARD_IDS"                  = "true"
}

@neelance
Copy link
neelance commented Apr 2, 2024

I'm seeing the same issue. Could it be because the named Firestore instance is not located in nam5?

@IchordeDionysos
Copy link
Contributor

@neelance ours is also not in nam5.

We have gotten it to work somehow, we didn't make any huge changes (only updated from version 0.1.46 to 0.1.48)

I guess we also did the following:

  1. Run terraform apply
  2. Get error that non-default database is not supported
  3. Run terraform plan
  4. Run terraform apply (Then it worked)

@neelance
Copy link
neelance commented Apr 3, 2024

@IchordeDionysos Are you sure that it actually worked? I also got a success message of the extension when I applied the config a second time, but the underlying Cloud Function was still in an error state and not working.

@IchordeDionysos
Copy link
Contributor

@neelance ahh you are correct... Same for us ...

We were just scrambling with a different error to get BigQuery setup properly as there's an issue when you enable partitioning:
#2028

@mikeymace
Copy link
mikeymace commented May 10, 2024

@jauntybrain What is the status of this issue?

I have just tested 0.1.48 and 0.1.49 and experience the same as @IchordeDionysos and @neelance

I have also left a comment on this issue: #2072 (as I didn't see this issue earlier)

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.

🐛 [firestore-bigquery-export] Select firestore database
8 participants