Uh oh!
There was an error while loading. Please reload this page.
Bigtable: 02. Surface: Readrows - #2849
Conversation
454512e to
58c62b0Compare58c62b0 to
1eb9a51Compareigorbernstein2
commented
Feb 8, 2018
rebased |
Adding the user facing api for ReadRows: - Adds wrappers for the ReadRowsRequest and Response - Introduces a fluent DSL for filters - Introduces the custom RowAdapter api where the user can customize the logical rows without introducing extra copies
d18ab82 to
90cc127Compare| * // Do something with row | ||
| * } | ||
| * | ||
| * // Point look ups |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| import java.util.List; | ||
| /** | ||
| * An extension point that allows end users to plugin a custom implementation of logical rows. This |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| /** | ||
| * An extension point that allows end users to plugin a custom implementation of logical rows. This | ||
| * useful in cases where the user would like to apply advanced client side filtering of cells. This | ||
| * adapter acts like a factory for SAX style a row builder. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| /** Returns the row key */ | ||
| @Nonnull | ||
| public abstract ByteString key(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| /** Default representation of a cell in a {@link Row}. */ | ||
| @AutoValue | ||
| public abstract static class Cell { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| /** | ||
| * A Fluent DSL to create a hierarchy of filters for the CheckAndMutateRow RPCs and ReadRows Query. | ||
| * | ||
| * <p>Intended usage is to statically import, or in case of conflict assign the static variable |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| // Per-method settings using baseSettings for defaults. | ||
| readRowsSettings = ServerStreamingCallSettings.newBuilder(); | ||
| /* TODO: copy timeouts, retryCodes & retrySettings from baseSettings.readRows once it exists in GAPIC */ |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| stub.close(); | ||
| } | ||
| /** |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| * @see Query For query options. | ||
| * @see com.google.cloud.bigtable.data.v2.wrappers.Filters For the filter building DSL. | ||
| */ | ||
| public <RowT> ServerStreamingCallable<Query, RowT> readRowsCallable(RowAdapter<RowT> rowAdapter) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| /** {@inheritDoc} */ | ||
| @Override | ||
| public void cellValue(ByteString value) { | ||
| this.value = this.value.concat(value); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
kevinsi4508
left a comment
There was a problem hiding this comment.
Some more comments. It is a Big PR.
| /** | ||
| * Constructs a new Query object for the specified table id. The table id will be resolved against | ||
| * the instance id specified in the {@link | ||
| * com.google.cloud.bigtable.data.v2.BigtableDataSettings}. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| public Query range(ByteStringRange range) { | ||
| RowRange.Builder rangeBuilder = RowRange.newBuilder(); | ||
| switch (range.getStartBound()) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| rangeBuilder.setStartKeyClosed(range.getStart()); | ||
| break; | ||
| default: | ||
| throw new IllegalStateException("Unknown range bound: " + range.getStartBound()); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| rangeBuilder.setEndKeyClosed(range.getEnd()); | ||
| break; | ||
| default: | ||
| throw new IllegalStateException("Unknown range bound: " + range.getEndBound()); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| } | ||
| /** | ||
| * Sets the filter to look apply to each row. Only one filter can be set at a time. To use |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| return builder | ||
| .setTableName(tableName.toString()) | ||
| .setAppProfileId(requestContext.getAppProfileId()) | ||
| .build(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| public enum BoundType { | ||
| OPEN, | ||
| CLOSED, | ||
| UNBOUNDED |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| this.start = start; | ||
| this.startBound = startBound; | ||
| this.end = end; | ||
| this.endBound = endBound; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| /** | ||
| * Creates a new {@link Range} with the specified inclusive start and the specified exclusive end. | ||
| */ | ||
| public R of(T startClosed, T endOpen) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| import com.google.protobuf.ByteString; | ||
| /** | ||
| * Range API. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
igorbernstein2
commented
Feb 9, 2018
Thanks for reviewing! I think addressed all feedback, PTAL |
garrettjonesgoogle
left a comment
There was a problem hiding this comment.
@pongad , could you take a look too?
| case UNBOUNDED: | ||
| break; | ||
| default: | ||
| throw new IllegalStateException("Unknown end bound: " + getStartBound()); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| // Extracted from: re2 QuoteMeta: | ||
| // https://github.com/google/re2/blob/70f66454c255080a54a8da806c52d1f618707f8a/re2/re2.cc#L456 | ||
| private static void writeLiteralRegex(ByteIterator input, OutputStream output) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| import java.io.OutputStream; | ||
| @InternalApi | ||
| public final class RegexUtil { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
igorbernstein2
commented
Feb 12, 2018
All feedback addressed. Please merge when ready |
🤖 I have created a release *beep* *boop* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
* test: clean up instance in test Change-Id: Ib3a73e2cc24f7e27fd346eb80b7ec5175e3c11a4 * format Change-Id: I41393dc1a87fffa0e8ec72d791a80f6e1434ab65
Adding the user facing api for ReadRows:
This has been reviewed Kevin. However some things have changed since the original review: