Repository navigation
feat: Session support for mutateRows - #14644
alexoneill wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for mutateRows (bulk mutations) across the Bigtable client's async API and compatibility layers, including TableAsync, AuthorizedViewAsync, and TableBase, along with a new MutateRowsShim implementation. The review feedback suggests importing com.google.common.base.Preconditions and adding a defensive null check for the client parameter in the MutateRowsShim constructor to ensure object integrity.
| import com.google.cloud.bigtable.data.v2.models.TargetId; | ||
| import io.grpc.Deadline; |
There was a problem hiding this comment.
Import com.google.common.base.Preconditions to support defensive null-checking of parameters in the constructor.
| import com.google.cloud.bigtable.data.v2.models.TargetId; | |
| import io.grpc.Deadline; | |
| import com.google.cloud.bigtable.data.v2.models.TargetId; | |
| import com.google.common.base.Preconditions; | |
| import io.grpc.Deadline; |
| public MutateRowsShim(Client client) { | ||
| tables = |
There was a problem hiding this comment.
Add a defensive null check for the client parameter in the constructor to ensure the object is initialized with a valid reference.
public MutateRowsShim(Client client) {
Preconditions.checkNotNull(client, "client is required");
tables =References
- Defensive null checks in constructors are appropriate to ensure object integrity, provided they are not redundant with upstream validation.
| return f; | ||
| } | ||
|
|
||
| // TODO: get deadline from compatibility layer |
There was a problem hiding this comment.
this comment can be removed. My PR is removing it in all other places.
| return true; | ||
| } | ||
|
|
||
| public static boolean isIdempotent(SessionMutateRowsRequest req) { |
There was a problem hiding this comment.
Can we remove this method?
on line 192 in tableBase you can do something like:
ImmutableList<Mutation> mutations = request.getEntriesList().stream().flatMap(entry -> entry.getMutationsList().stream()).collect(toImmutableList());
Util.isIdempotent(mutations);
| private final SessionPoolMap<AuthorizedViewId, AuthorizedViewAsync> authViews; | ||
|
|
||
| public MutateRowsShim(Client client) { | ||
| tables = |
There was a problem hiding this comment.
we should consider share the pool with MutateRow.
See my PR (although is not approved yet 😂 )
https://github.com/googleapis/google-cloud-java/pull/14585/changes#diff-e47339d2678dd2991014d93d7eaa96d999bb53d535c753e67766f0582b2212f4
No description provided.