We removed error feedback? So how is an application programmer supposed to determine what failed now? Without that information we may have rendered putAll unusable for some cases.
-- Mike Stolz Principal Engineer, GemFire Product Manager Mobile: +1-631-835-4771 On Wed, Jul 12, 2017 at 1:45 PM, Brian Rowe <br...@pivotal.io> wrote: > > > > On July 7, 2017, 11:51 p.m., Udo Kohlmeyer wrote: > > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/operations/PutAllRequestOperationHandler.java > > > Lines 81 (patched) > > > <https://reviews.apache.org/r/60718/diff/1/?file=1771594# > file1771594line81> > > > > > > I'm sure that we can use `region.putAll(entries)` for this. > > Fixed. This also resulted in us not being able to determine the state of > a partially succeeded PutAll, so we updated the PutAllResponse to remove > the failedKeys field. > > > > On July 7, 2017, 11:51 p.m., Udo Kohlmeyer wrote: > > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/utilities/ProtobufRequestUtilities.java > > > Lines 76 (patched) > > > <https://reviews.apache.org/r/60718/diff/1/?file=1771595# > file1771595line76> > > > > > > maybe this variable name should be `getAllRequestBuilder`... not > advocate of generic `builder` > > Fixed > > > > On July 7, 2017, 11:51 p.m., Udo Kohlmeyer wrote: > > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/utilities/ProtobufRequestUtilities.java > > > Lines 78-80 (patched) > > > <https://reviews.apache.org/r/60718/diff/1/?file=1771595# > file1771595line78> > > > > > > Could we replace this with `addAllKey(java.lang.Iterable<? > extends org.apache.geode.protocol.protobuf.BasicTypes.EncodedValue> > values)` > > Nice, good catch. > > > > On July 7, 2017, 11:51 p.m., Udo Kohlmeyer wrote: > > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/utilities/ProtobufRequestUtilities.java > > > Lines 95-97 (patched) > > > <https://reviews.apache.org/r/60718/diff/1/?file=1771595# > file1771595line95> > > > > > > Could we replace this with `addAllEntry(java.lang.Iterable<? > extends org.apache.geode.protocol.protobuf.BasicTypes.Entry> values)` > > Fixed > > > > On July 7, 2017, 11:51 p.m., Udo Kohlmeyer wrote: > > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/utilities/ProtobufResponseUtilities.java > > > Lines 125-127 (patched) > > > <https://reviews.apache.org/r/60718/diff/1/?file=1771596# > file1771596line125> > > > > > > Could we replace this with `addAllEntries(java.lang.Iterable<? > extends org.apache.geode.protocol.protobuf.BasicTypes.Entry> values)` > > Fixed > > > > On July 7, 2017, 11:51 p.m., Udo Kohlmeyer wrote: > > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/utilities/ProtobufResponseUtilities.java > > > Lines 141-143 (patched) > > > <https://reviews.apache.org/r/60718/diff/1/?file=1771596# > file1771596line141> > > > > > > could we possibly use `addAllFailedKeys(java.lang.Iterable<? > extends org.apache.geode.protocol.protobuf.BasicTypes.EncodedValue> > values)` > > This got removed with the failed keys field. > > > - Brian > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60718/#review179967 > ----------------------------------------------------------- > > > On July 7, 2017, 9:18 p.m., Brian Rowe wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/60718/ > > ----------------------------------------------------------- > > > > (Updated July 7, 2017, 9:18 p.m.) > > > > > > Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen > O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer. > > > > > > Bugs: GEODE-2997 > > https://issues.apache.org/jira/browse/GEODE-2997 > > > > > > Repository: geode > > > > > > Description > > ------- > > > > Changed get response to indicate if LookupFailure was a missing key or > key with null value, added test > > Added GetAllRequestOperationHandler and unit test > > Added PutAllRequestOperationHandler and unit test > > Added an integration test covering the putAll and getAll operations > > > > > > Diffs > > ----- > > > > > > geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java > ebd5c6a0a > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/operations/GetAllRequestOperationHandler.java PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/operations/PutAllRequestOperationHandler.java PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/utilities/ProtobufRequestUtilities.java b96f478d1 > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/utilities/ProtobufResponseUtilities.java 2114fdbf7 > > geode-protobuf/src/main/java/org/apache/geode/protocol/ > protobuf/utilities/ProtobufUtilities.java 924979329 > > geode-protobuf/src/test/java/org/apache/geode/protocol/ > RoundTripCacheConnectionJUnitTest.java 31a873658 > > geode-protobuf/src/test/java/org/apache/geode/protocol/ > protobuf/operations/GetAllRequestOperationHandlerJUnitTest.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/protocol/ > protobuf/operations/GetRequestOperationHandlerJUnitTest.java b7d52019e > > geode-protobuf/src/test/java/org/apache/geode/protocol/ > protobuf/operations/PutAllRequestOperationHandlerJUnitTest.java > PRE-CREATION > > > > > > Diff: https://reviews.apache.org/r/60718/diff/1/ > > > > > > Testing > > ------- > > > > Added unit tests for new operation handlers > > Added integration test covering new operations > > > > > > Thanks, > > > > Brian Rowe > > > > > >
