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(pubsub): Add Schema support #9305

Merged
merged 27 commits into from
Mar 30, 2021
Merged

feat(pubsub): Add Schema support #9305

merged 27 commits into from
Mar 30, 2021

Conversation

quartzmo
Copy link
Member

@quartzmo quartzmo commented Feb 8, 2021

closes: #9304

@product-auto-label product-auto-label bot added the api: pubsub Issues related to the Pub/Sub API. label Feb 8, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 8, 2021
@quartzmo quartzmo self-assigned this Feb 8, 2021
@quartzmo quartzmo force-pushed the pubsub-schema branch 2 times, most recently from e51cbac to 989d981 Compare February 17, 2021 19:14
@quartzmo quartzmo force-pushed the pubsub-schema branch 3 times, most recently from 7bcf49f to d8de2eb Compare February 25, 2021 20:41
@quartzmo quartzmo force-pushed the pubsub-schema branch 2 times, most recently from 4ab3ada to dff3d90 Compare March 11, 2021 23:08
@snippet-bot
Copy link

snippet-bot bot commented Mar 13, 2021 •
edited

Here is the summary of changes.

You are about to add 12 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@quartzmo quartzmo marked this pull request as ready for review March 20, 2021 00:04
@quartzmo quartzmo requested a review from a team as a code owner March 20, 2021 00:04
@quartzmo
Copy link
Member Author

Samples - Linux failure: 1 ruby version out of 4 failed with the following flake unrelated to this PR:

subscriptions#test_0007_supports pubsub_subscriber_sync_pull_with_lease:
NoMethodError: undefined method `modify_ack_deadline!' for nil:NilClass
    /tmpfs/src/github/google-cloud-ruby/google-cloud-pubsub/samples/subscriptions.rb:349:in `block in subscriber_sync_pull_with_lease'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-pubsub/samples/subscriptions.rb:339:in `loop'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-pubsub/samples/subscriptions.rb:339:in `subscriber_sync_pull_with_lease'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-pubsub/samples/acceptance/subscriptions_test.rb:159:in `block (4 levels) in <top (required)>'

@quartzmo quartzmo added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 22, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 22, 2021
anguillanneuf
anguillanneuf approved these changes Mar 22, 2021
View reviewed changes
Copy link

@anguillanneuf anguillanneuf left a comment

Choose a reason for hiding this comment

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

I reviewed the samples:

  • google-cloud-pubsub/samples/acceptance/data/*
  • google-cloud-pubsub/samples/utilities/us-states_pb.rb
  • google-cloud-pubsub/samples/schemas.rb
  • google-cloud-pubsub/samples/acceptance/schemas_test.rb: thank you for adding tests for all the different scenarios!

@quartzmo
Copy link
Member Author

@anguillanneuf Thank you!

dazuma
dazuma approved these changes Mar 25, 2021
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

google-cloud-pubsub/lib/google/cloud/pubsub/service.rb Show resolved Hide resolved
@quartzmo quartzmo added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 29, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 29, 2021
kamalaboulhosn
kamalaboulhosn requested changes Mar 29, 2021
View reviewed changes
google-cloud-pubsub/google-cloud-pubsub.gemspec
@@ -23,6 +23,7 @@ Gem::Specification.new do |gem|
gem.add_dependency "google-cloud-pubsub-v1", "~> 0.0"

gem.add_development_dependency "autotest-suffix", "~> 1.1"
gem.add_development_dependency "avro", "~> 1.10"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this introducing the dependency because of the tests and/or samples? I'm a little nervous introducing the dependency for the entire library just because of those. For example, one who is using proto isn't going to want to have a dependency on the avro library.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just for tests in this case, and won't impact the published package.

Development dependencies aren't installed by default and aren't activated when a gem is required.

https://guides.rubygems.org/specification-reference/#add_development_dependency

google-cloud-pubsub/lib/google/cloud/pubsub/project.rb Outdated
# than the one currently connected to, the alternate project ID can be
# specified here. Not used if a fully-qualified schema name is
# provided for `schema_name`.
# @param [Boolean] skip_lookup Optionally create a {Subscription} object
Copy link
Contributor

Choose a reason for hiding this comment

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

Subscription -> Schema throughout this file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, thanks!

@quartzmo
Copy link
Member Author

OSx build failing with:

********************************
*** google-cloud-pubsub test ***
********************************

bundle exec yard config load_plugins true && bundle exec yard doctest

File does not exist: grpc/grpc_c

rake aborted!
Command failed with status (1)
/Volumes/BuildData/tmpfs/src/github/google-cloud-ruby/google-cloud-pubsub/Rakefile:184:in `block in <top (required)>'
/Users/kbuilder/.rvm/rubies/ruby-3.0.0/bin/ruby_executable_hooks:22:in `eval'
/Users/kbuilder/.rvm/rubies/ruby-3.0.0/bin/ruby_executable_hooks:22:in `<main>'

@quartzmo quartzmo merged commit 3c91de7 into googleapis:master Mar 30, 2021
@quartzmo quartzmo deleted the pubsub-schema branch March 30, 2021 17:39
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

@kamalaboulhosn kamalaboulhosn kamalaboulhosn approved these changes

@anguillanneuf anguillanneuf anguillanneuf approved these changes

Assignees

@quartzmo quartzmo

Labels
api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expose SchemaService in handwritten layer
5 participants
@quartzmo @dazuma @kamalaboulhosn @anguillanneuf @yoshi-kokoro