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.

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

feat: add an enhanced layer for BigQuery Storage v1beta2 client #48

Merged
merged 4 commits into from Jan 30, 2020
Merged

feat: add an enhanced layer for BigQuery Storage v1beta2 client #48

merged 4 commits into from Jan 30, 2020

Conversation

mmladenovski
Copy link
Contributor

This pull request creates an enhanced shim layer in the BigQuery Storage API v1beta2 client in order to apply a streaming resumption strategy for the ReadRows method.

  • First commit creates the enhanced shim layer
  • Second commit adds the custom streaming resumption strategy

The changes are similar to the PR for v1beta1: googleapis/google-cloud-java#4022. The difference is that a lot of the timeout and retry settings are now automatically applied on the base (auto-generated) client.

Some tips on how to review the PR and which files to compare:

  • EnhancedBigQueryReadStub.java is based from BigQueryReadStub.java
  • EnhancedBigQueryReadStubSettings.java is based from BigQueryReadStubSettings.java
  • BigQueryReadSettings.java is based from BaseBigQueryReadSettings.java
  • BigQueryReadClient.java is based from BaseBigQueryReadClient.java

@kmjung , @emkornfield, @aryann please review the changes.

This change adds a simple shim layer to the BigQuery Read API client.

It is modeled after the shim layer in the BigQuery Storage API v1beta1, where the parameters are simple pass-throughs to the underlying gRPC client stub.

The shim currently does nothing but read default values from the base
client layer and pass them back through.
This change modifies the configuration for the v1beta2 ReadRows API to add a resumption strategy. The resumption strategy will allow the connection to be resumed transparently in the case of transient errors.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 29, 2020
@mmladenovski mmladenovski changed the title Add an enhanced layer for BigQuery Storage v1beta2 client. feat: add an enhanced layer for BigQuery Storage v1beta2 client. Jan 29, 2020
@kmjung
Copy link
Contributor

kmjung commented Jan 29, 2020

This whole "enhanced" infrastructure is just, at this point, to allow us to specify a resumption strategy in the default ReadRows settings? I'd really love to get away from this. @chingor13 @shollyman @stephaniewang526 -- any way we can get the codegen tool to do this for us?

@mmladenovski mmladenovski changed the title feat: add an enhanced layer for BigQuery Storage v1beta2 client. feat: add an enhanced layer for BigQuery Storage v1beta2 client Jan 29, 2020
@emkornfield
Copy link
Contributor

@mmladenovski should I wait to review until @kmjung's question gets answered?

@mmladenovski
Copy link
Contributor Author

@mmladenovski should I wait to review until @kmjung's question gets answered?

@emkornfield, please go ahead with a review. I am not sure how long it will take for automated resumption strategy generation.

@emkornfield
Copy link
Contributor

Took a look at manual diffs provided offline and perused the code as is, overall looks OK to me.

@chingor13 chingor13 requested review from stephaniewang526 and removed request for chingor13 January 29, 2020 21:31
@stephaniewang526 stephaniewang526 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 30, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 30, 2020
@codecov
Copy link

codecov bot commented Jan 30, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@8c124a2). Click here to learn what that means.
The diff coverage is 65.06%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #48   +/-   ##
=========================================
  Coverage          ?   71.11%           
  Complexity        ?      197           
=========================================
  Files             ?       28           
  Lines             ?     1357           
  Branches          ?        3           
=========================================
  Hits              ?      965           
  Misses            ?      389           
  Partials          ?        3
Impacted Files Coverage Δ Complexity Δ
...eta2/stub/readrows/ReadRowsResumptionStrategy.java 100% <100%> (ø) 5 <5> (?)
...bigquery/storage/v1beta2/BigQueryReadSettings.java 23.52% <23.52%> (ø) 3 <3> (?)
...d/bigquery/storage/v1beta2/BigQueryReadClient.java 52.94% <52.94%> (ø) 9 <9> (?)
...v1beta2/stub/EnhancedBigQueryReadStubSettings.java 81.25% <81.25%> (ø) 8 <8> (?)
...storage/v1beta2/stub/EnhancedBigQueryReadStub.java 82.92% <82.92%> (ø) 7 <7> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c124a2...45970e6. Read the comment docs.

@stephaniewang526 stephaniewang526 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 30, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 30, 2020
@stephaniewang526 stephaniewang526 merged commit 9496158 into googleapis:master Jan 30, 2020
@mmladenovski mmladenovski deleted the v1beta2 branch January 30, 2020 21:03
shubhwip pushed a commit to shubhwip/java-bigquerystorage that referenced this pull request Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants