Skip to content

chore(firestore): Add query watch samples#7602

Merged
quartzmo merged 8 commits into
googleapis:masterfrom
quartzmo:firestore-samples-query-watch
Sep 30, 2020
Merged

chore(firestore): Add query watch samples#7602
quartzmo merged 8 commits into
googleapis:masterfrom
quartzmo:firestore-samples-query-watch

Conversation

@quartzmo
Copy link
Copy Markdown
Member

@quartzmo quartzmo commented Aug 28, 2020 •
edited
Loading

Known region tags:

  • fs_listen_changes
  • fs_listen_document
  • fs_listen_errors
  • fs_listen_multiple

closes: #7595

@quartzmo quartzmo requested a review from a team August 28, 2020 22:48
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 28, 2020
@quartzmo quartzmo self-assigned this Aug 28, 2020
@quartzmo quartzmo added api: firestore Issues related to the Firestore API. type: docs Improvement to the documentation for an API. labels Aug 28, 2020
@quartzmo
Copy link
Copy Markdown
Member Author

quartzmo commented Aug 28, 2020 •
edited
Loading

Regarding the region tag fs_listen_errors (as demonstrated in golang-samples/firestore/firestore_snippets/listen_errors.go), I'm not sure how to provide this sample for Ruby google-cloud-firestore.

@quartzmo
Copy link
Copy Markdown
Member Author

quartzmo commented Aug 31, 2020 •
edited
Loading

@schmidt-sebastian @dazuma The google-cloud-firestore Ruby client does not report errors from watch threads. Other clients in google-cloud-ruby, such as those for debugger, error_reporting, logger, pubsub, etc) do accept on_error callbacks to surface errors from worker threads. (Example: PubSub::Subscriber#on_error). Should we add this feature to DocumentListener and QueryListener?

@quartzmo quartzmo changed the title docs(firestore): Add query watch samples chore(firestore): Add query watch samples Sep 3, 2020
schmidt-sebastian
schmidt-sebastian reviewed Sep 8, 2020
View reviewed changes
Comment thread google-cloud-firestore/samples/query_watch.rb Outdated
@schmidt-sebastian
Copy link
Copy Markdown

schmidt-sebastian commented Sep 8, 2020

@schmidt-sebastian @dazuma The google-cloud-firestore Ruby client does not report errors from watch threads. Other clients in google-cloud-ruby, such as those for debugger, error_reporting, logger, pubsub, etc) do accept on_error callbacks to surface errors from worker threads. (Example: PubSub::Subscriber#on_error). Should we add this feature to DocumentListener and QueryListener?

We do need to raise errors as not all backend errors are handled transparently.

@quartzmo quartzmo mentioned this pull request Sep 10, 2020
Closed
@quartzmo quartzmo force-pushed the firestore-samples-query-watch branch from 56e194c to cd7a423 Compare September 11, 2020 23:11
@quartzmo
Copy link
Copy Markdown
Member Author

quartzmo commented Sep 21, 2020

This PR should wait for #7811.

@quartzmo quartzmo added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 21, 2020
@quartzmo quartzmo force-pushed the firestore-samples-query-watch branch from cd7a423 to 668d8f0 Compare September 24, 2020 21:54
@quartzmo quartzmo added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Sep 24, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 24, 2020
@quartzmo
Copy link
Copy Markdown
Member Author

quartzmo commented Sep 24, 2020

@schmidt-sebastian @dazuma This PR is ready for review.

schmidt-sebastian
schmidt-sebastian reviewed Sep 24, 2020
View reviewed changes
Copy link
Copy Markdown

@schmidt-sebastian schmidt-sebastian left a comment

Choose a reason for hiding this comment

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

Some comments to prove that I don't understand Ruby :)

Comment thread google-cloud-firestore/samples/acceptance/query_watch_test.rb Outdated
Comment thread google-cloud-firestore/samples/query_watch.rb Outdated
Comment thread google-cloud-firestore/samples/query_watch.rb Outdated
Comment thread google-cloud-firestore/samples/query_watch.rb Outdated
Comment thread google-cloud-firestore/samples/query_watch.rb Outdated
Comment thread google-cloud-firestore/samples/query_watch.rb Outdated
Comment thread google-cloud-firestore/samples/query_watch.rb Outdated
dazuma
dazuma approved these changes Sep 25, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

Looks great overall. Just a couple nits and questions.

Comment thread google-cloud-firestore/samples/acceptance/query_watch_test.rb Outdated
Comment thread google-cloud-firestore/samples/acceptance/query_watch_test.rb Outdated
Comment thread google-cloud-firestore/samples/query_watch.rb Outdated
Comment thread google-cloud-firestore/samples/query_watch.rb Outdated
@quartzmo quartzmo force-pushed the firestore-samples-query-watch branch from 64d3507 to e351021 Compare September 29, 2020 22:31
schmidt-sebastian
schmidt-sebastian reviewed Sep 30, 2020
View reviewed changes
Comment thread google-cloud-firestore/samples/query_watch.rb Outdated
firestore = Google::Cloud::Firestore.new project_id: project_id
# [START fs_listen_multiple]
query = firestore.col(collection_path).where :state, :==, "CA"
snapshots = []
Copy link
Copy Markdown

@schmidt-sebastian schmidt-sebastian Sep 30, 2020

Choose a reason for hiding this comment

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

s/snapshots/docs

@quartzmo quartzmo merged commit bb6acb8 into googleapis:master Sep 30, 2020
@quartzmo quartzmo deleted the firestore-samples-query-watch branch September 30, 2020 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@dazuma dazuma dazuma approved these changes

@schmidt-sebastian schmidt-sebastian schmidt-sebastian approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@quartzmo quartzmo

Labels

api: firestore Issues related to the Firestore API. cla: yes This human has signed the Contributor License Agreement. type: docs Improvement to the documentation for an API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

firestore: Add samples for query watch

4 participants

@quartzmo @schmidt-sebastian @dazuma @yoshi-kokoro