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.

Sign up for GitHub

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

Jump to bottom

feat(firestore): Added support for multiple databases #20029

Merged
merged 5 commits into from Feb 9, 2023
Merged

feat(firestore): Added support for multiple databases #20029

merged 5 commits into from Feb 9, 2023

Conversation

diptanshumittal
Copy link
Contributor

No description provided.

@diptanshumittal diptanshumittal requested a review from a team as a code owner January 16, 2023 08:32
@product-auto-label product-auto-label bot added the api: firestore Issues related to the Firestore API. label Jan 16, 2023
@diptanshumittal diptanshumittal changed the title feaet(firestore): Added support for multiple databases [WIP] feat(firestore): Added support for multiple databases Jan 16, 2023
@diptanshumittal diptanshumittal added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 16, 2023
@diptanshumittal diptanshumittal marked this pull request as draft January 16, 2023 08:34
@diptanshumittal diptanshumittal added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@diptanshumittal diptanshumittal added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@NivedhaSenthil NivedhaSenthil added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@NivedhaSenthil NivedhaSenthil added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@NivedhaSenthil NivedhaSenthil added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 17, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@diptanshumittal diptanshumittal added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 17, 2023
@diptanshumittal diptanshumittal force-pushed the feat_firestore_multi_db branch 2 times, most recently from 5354ed0 to b10eba4 Compare January 18, 2023 04:07
@diptanshumittal diptanshumittal changed the title [WIP] feat(firestore): Added support for multiple databases feat(firestore): Added support for multiple databases Jan 18, 2023
@diptanshumittal diptanshumittal removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 18, 2023
@diptanshumittal diptanshumittal marked this pull request as ready for review January 18, 2023 13:00
dazuma
dazuma reviewed Jan 24, 2023
View reviewed changes
google-cloud-firestore/lib/google/cloud/firestore/service.rb
@@ -48,7 +50,7 @@ def firestore
config.endpoint = host if host
config.lib_name = "gccl"
config.lib_version = Google::Cloud::Firestore::VERSION
config.metadata = { "google-cloud-resource-prefix": "projects/#{@project}/databases/(default)" }
config.metadata = { "google-cloud-resource-prefix": "projects/#{@project}/databases/#{@database}" }
Copy link
Member

Choose a reason for hiding this comment

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

What if @database is nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Error will be thrown in this case. But @database can be nil only if the user explicity initialize an object of Service class, which is marked as private, and use it for sending requests.
While initializing the firestore client, @database will have the the value (default) unless overridden by the user.

dazuma
dazuma approved these changes Feb 8, 2023
View reviewed changes
Copy link
Member

@dazuma dazuma left a comment

Choose a reason for hiding this comment

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

LGTM

@diptanshumittal diptanshumittal merged commit 93bacdf into main Feb 9, 2023
@diptanshumittal diptanshumittal deleted the feat_firestore_multi_db branch February 9, 2023 00:51
@yoshi-code-bot yoshi-code-bot mentioned this pull request Feb 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dazuma dazuma dazuma approved these changes

Assignees
No one assigned
Labels
api: firestore Issues related to the Firestore API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@diptanshumittal @dazuma @NivedhaSenthil @yoshi-kokoro