-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[cloud_functions]Add region support for cloud functions #1053
[cloud_functions]Add region support for cloud functions #1053
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
|
I signed it! |
|
CLAs look good, thanks! |
|
Any updates or feedback is appreciated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Knupper I checked FirebaseApp hoping we got a default region for the project through in google-services.json but we don't. With Firestore now going multi-region it might be worth combining efforts here if there is a more direct route.
Other change that might be worth making is that FirebaseApp should be able to be passed in. This might already be in another PR. Might be worth making another PR for that however.
References to how Firestore plugin does it.
https://github.com/flutter/plugins/blob/master/packages/cloud_firestore/lib/src/firestore.dart#L132
https://github.com/flutter/plugins/blob/master/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java#L80
https://firebase.google.com/docs/reference/android/com/google/firebase/functions/FirebaseFunctions.html#getInstance%28com.google.firebase.FirebaseApp%2C+java.lang.String%29
|
@kroikie hey Arthur, Needs a second review and I nominate you 😁 If your happy with it, then I leave it for you to merge and release 👍 |
|
@slightfoot can be closed, we have region support now :) |
|
Looks like PR #1210 has superseded this one with support for multiple Apps and Regions. Closing. |
|
Wait, so do we have region support for cloud functions plug in now? If it is, it is super unclear as to how to call functions in different regions. |
Hey,
here my implementation to add region support for cloud_functions (see #24333).
King regards
Max