[
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)
