We will still return ErrorResponses for any failures or invalid messages, we've just removed the serverInternal and retriable fields from this message in favor a errorCode integer.
On Wed, Jul 12, 2017 at 11:45 AM, Michael Stolz <mst...@pivotal.io> wrote: > 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 <(631)%20835-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/proto >> buf/operations/PutAllRequestOperationHandler.java >> > > Lines 81 (patched) >> > > <https://reviews.apache.org/r/60718/diff/1/?file=1771594#fil >> e1771594line81> >> > > >> > > 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/proto >> buf/utilities/ProtobufRequestUtilities.java >> > > Lines 76 (patched) >> > > <https://reviews.apache.org/r/60718/diff/1/?file=1771595#fil >> e1771595line76> >> > > >> > > 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/proto >> buf/utilities/ProtobufRequestUtilities.java >> > > Lines 78-80 (patched) >> > > <https://reviews.apache.org/r/60718/diff/1/?file=1771595#fil >> e1771595line78> >> > > >> > > 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/proto >> buf/utilities/ProtobufRequestUtilities.java >> > > Lines 95-97 (patched) >> > > <https://reviews.apache.org/r/60718/diff/1/?file=1771595#fil >> e1771595line95> >> > > >> > > 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/proto >> buf/utilities/ProtobufResponseUtilities.java >> > > Lines 125-127 (patched) >> > > <https://reviews.apache.org/r/60718/diff/1/?file=1771596#fil >> e1771596line125> >> > > >> > > 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/proto >> buf/utilities/ProtobufResponseUtilities.java >> > > Lines 141-143 (patched) >> > > <https://reviews.apache.org/r/60718/diff/1/?file=1771596#fil >> e1771596line141> >> > > >> > > 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/prot >> obuf/ProtobufStreamProcessor.java ebd5c6a0a >> > geode-protobuf/src/main/java/org/apache/geode/protocol/prot >> obuf/operations/GetAllRequestOperationHandler.java PRE-CREATION >> > geode-protobuf/src/main/java/org/apache/geode/protocol/prot >> obuf/operations/PutAllRequestOperationHandler.java PRE-CREATION >> > geode-protobuf/src/main/java/org/apache/geode/protocol/prot >> obuf/utilities/ProtobufRequestUtilities.java b96f478d1 >> > geode-protobuf/src/main/java/org/apache/geode/protocol/prot >> obuf/utilities/ProtobufResponseUtilities.java 2114fdbf7 >> > geode-protobuf/src/main/java/org/apache/geode/protocol/prot >> obuf/utilities/ProtobufUtilities.java 924979329 >> > geode-protobuf/src/test/java/org/apache/geode/protocol/Roun >> dTripCacheConnectionJUnitTest.java 31a873658 >> > geode-protobuf/src/test/java/org/apache/geode/protocol/prot >> obuf/operations/GetAllRequestOperationHandlerJUnitTest.java PRE-CREATION >> > geode-protobuf/src/test/java/org/apache/geode/protocol/prot >> obuf/operations/GetRequestOperationHandlerJUnitTest.java b7d52019e >> > geode-protobuf/src/test/java/org/apache/geode/protocol/prot >> obuf/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 >> > >> > >> >> >
