8000 Fixes an issue with splitReadStream by aryann · Pull Request #6112 · googleapis/google-cloud-java · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@aryann
Copy link
@aryann aryann commented Aug 20, 2019

The method requires a fraction, which is not provided in this client.

Additionally, I'm removing some comments that don't make sense.

aryann added 2 commits August 20, 2019 11:36
All of these methods were handwritten.
The server denies requests without a fractional value set. This change sets a
semi-sensible default. In the future we should remove this method and force
users to use the one with a request message since users should know what
fraction they want to split at (i.e., 0.5 is not always a good choice).
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 20, 2019
@chingor13
Copy link
Contributor

Thanks for submitting this. The comments mean that this entire file is managed by a code generator, so any hand-written updates will be overwritten. We will need to address this upstream in our generator and/or service definitions.

Can you open an issue here to reference and we will look into how to best make the changes upstream?

@aryann
Copy link
Author
aryann commented Aug 21, 2019

I was told my @mmladenovski that this file was written by hand, which is why I removed the comments.

How is this file generated?

@mmladenovski
Copy link
mmladenovski commented Aug 21, 2019

I was told my @mmladenovski that this file was written by hand, which is why I removed the comments.

How is this file generated?

@aryann , the file was added with #4022 . @kmjung , might provide more details.

I think it was copied from the auto-generated BaseBigQueryStorageClient and then modified to use the Enhanced stub along with the ResumptionStrategy. Currently the file is manually maintained.

The comments related to auto-generation should be removed.

@chingor13 chingor13 added the api: bigquery Issues related to the BigQuery API. label Aug 26, 2019
SplitReadStreamRequest.newBuilder().setOriginalStream(originalStream).build();
SplitReadStreamRequest.newBuilder()
.setOriginalStream(originalStream)
.setFraction(0.5f)

Choose a reason for hiding this comment

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

I would suggest to move the 0.5f value to a constant STREAM_SPLIT_FRACTION and use it in all places!

@meredithslota
Copy link

The BigQuery Storage client for Java now lives here: https://github.com/googleapis/java-bigquerystorage and has recently been re-generated. The comments re: auto-generation are still present because this is an autogenerated library, but if you see other issues (the file referenced from v1beta1 is here: https://github.com/googleapis/java-bigquerystorage/blob/master/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta1/BigQueryStorageClient.java) please file an issue in that repo. I'm going to go ahead and close this.

cc: @stephaniewang526 as an FYI and in case you want to do something different with this issue. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

0