Skip to content

feat(pubsub): Add Schema support#9305

Merged
quartzmo merged 27 commits into
googleapis:masterfrom
quartzmo:pubsub-schema
Mar 30, 2021
Merged

feat(pubsub): Add Schema support#9305
quartzmo merged 27 commits into
googleapis:masterfrom
quartzmo:pubsub-schema

Conversation

@quartzmo
Copy link
Copy Markdown
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
Copy Markdown

snippet-bot Bot commented Mar 13, 2021 •
edited
Loading

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 added 17 commits March 18, 2021 18:57
@quartzmo quartzmo marked this pull request as ready for review March 20, 2021 00:04
@quartzmo quartzmo requested review from a team and anguillanneuf March 20, 2021 00:04
@quartzmo
Copy link
Copy Markdown
Member Author

quartzmo commented Mar 22, 2021

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
Copy Markdown

@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
Copy Markdown
Member Author

quartzmo commented Mar 22, 2021

@anguillanneuf Thank you!

dazuma
dazuma suggested changes Mar 25, 2021
View reviewed changes
Comment thread google-cloud-pubsub/acceptance/pubsub/schema_test.rb Outdated
Comment thread google-cloud-pubsub/lib/google/cloud/pubsub/project.rb
Comment thread google-cloud-pubsub/lib/google/cloud/pubsub/schema.rb Outdated
Comment thread google-cloud-pubsub/lib/google/cloud/pubsub/schema.rb Outdated
Comment thread google-cloud-pubsub/lib/google/cloud/pubsub/schema.rb Outdated
Comment thread google-cloud-pubsub/lib/google/cloud/pubsub/service.rb
Comment thread google-cloud-pubsub/lib/google/cloud/pubsub/service.rb
Comment thread google-cloud-pubsub/lib/google/cloud/pubsub/service.rb Outdated
dazuma
dazuma approved these changes Mar 25, 2021
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.

LGTM

Comment thread google-cloud-pubsub/lib/google/cloud/pubsub/service.rb
@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
Comment thread google-cloud-pubsub/google-cloud-pubsub.gemspec
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
Copy Markdown
Contributor

@kamalaboulhosn kamalaboulhosn Mar 29, 2021

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
Copy Markdown
Member Author

@quartzmo quartzmo Mar 29, 2021

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

Comment thread 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
Copy Markdown
Contributor

@kamalaboulhosn kamalaboulhosn Mar 29, 2021

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
Copy Markdown
Member Author

@quartzmo quartzmo Mar 29, 2021

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
Copy Markdown
Member Author

quartzmo commented Mar 29, 2021

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

@kamalaboulhosn kamalaboulhosn kamalaboulhosn approved these changes

+2 more reviewers

@dazuma dazuma dazuma approved these changes

@anguillanneuf anguillanneuf anguillanneuf approved these changes

Reviewers whose approvals may not affect merge requirements

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