[ 
https://issues.apache.org/jira/browse/GEODE-3386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16122531#comment-16122531
 ] 

ASF GitHub Bot commented on GEODE-3386:
---------------------------------------

Github user WireBaron commented on a diff in the pull request:

    https://github.com/apache/geode/pull/700#discussion_r132591361
  
    --- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAllRequestOperationHandler.java
 ---
    @@ -59,13 +59,14 @@
           }
           return 
Success.of(RegionAPI.GetAllResponse.newBuilder().addAllEntries(entries).build());
         } catch (UnsupportedEncodingTypeException ex) {
    -      return Failure.of(BasicTypes.ErrorResponse.newBuilder()
    -          .setErrorCode(ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue)
    -          .setMessage("Encoding not supported.").build());
    +      int errorCode = ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue;
    +      String message = "Encoding not supported.";
    +      return 
Failure.of(ProtobufResponseUtilities.makeErrorResponse(errorCode, message));
    --- End diff --
    
    Having a local variable for the error code and message or inlining them are 
both fine approaches, but it's a bit jarring to see both approaches used in two 
adjacent code blocks.  Please use a consistent approach.


> Create Error type for KeyedErrorResponse and ErrorResponse
> ----------------------------------------------------------
>
>                 Key: GEODE-3386
>                 URL: https://issues.apache.org/jira/browse/GEODE-3386
>             Project: Geode
>          Issue Type: Sub-task
>          Components: client/server
>            Reporter: Galen O'Sullivan
>
> For logical separation of the new client API, it will be better to have an 
> Error that is contained by ErrorResponse, rather than having 
> KeyedErrorResponse contain an ErrorResponse.
> In pseudo-protobuf,
> {code}
> PutAllResponse {
>   repeated Entry successes = 1,
>   repeated KeyedErrorResponse errors = 2,
> }
> KeyedErrorResponse {
>   Key,
>   ErrorResponse
> }
> ErrorResponse {
>   string
> }
> {code}
> instead,
> {code}
> KeyedErrorResponse {
>   Key,
>   Error,
> }
> {code}
> and 
> {code}
> ErrorResponse {
> Error
> }
> Error {
> string
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to