Skip to content

Conversation

rahulKQL
Copy link
Owner

In Progress

@rahulKQL rahulKQL marked this pull request as ready for review April 30, 2020 14:59
Comment on lines 290 to 291
private <ReqT, RowT> ServerStreamingCallable<ReadRowsRequest, RowT> createReadRowsRawCallable(
ServerStreamingCallSettings<ReqT, Row> readRowsSettings, RowAdapter<RowT> rowAdapter) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Do you think is it acceptable for a private method to have two generic parameters but return the third class instead?

Because those generics classes would be only used for fetching the settings.

Choose a reason for hiding this comment

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

I don't see any problems with that

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks a lot for your opinion!!!

* <li>Upon receiving the response stream, it will merge the {@link
* com.google.bigtable.v2.ReadRowsResponse.CellChunk}s in logical rows. The actual row
* implementation can be configured in by the {@code rowAdapter} parameter.
* <li>Retry/resume on failure.

Choose a reason for hiding this comment

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

be configured in by
->
be configured by

(3 times)

@rahulKQL rahulKQL force-pushed the readRowsCallableOp branch 2 times, most recently from 86cbd43 to 8cb4be2 Compare April 30, 2020 15:30
…leStub

This commit would enable the user to target the table using absolute resource name on each read request. Currently we expose `ServerStreamingCallable<Query, RowT>`, which does not have an option to provide different `app-profile-id` on each request.
@rahulKQL rahulKQL force-pushed the readRowsCallableOp branch from 8cb4be2 to fb025ae Compare April 30, 2020 15:32
With this commit, the public createReadRowsCallable() would refer to single createReadRowsBaseCallable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants