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

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

jhutchison commented on pull request #5604:
URL: https://github.com/apache/geode/pull/5604#issuecomment-714528770


   > This PR changes way too many files. I didn't look at all of them. But it 
should have only changed 2 or 3.
   > I'm not sure I like this change because now we have the parameters in two 
places. We still have the switch statement in InfoExecutor that checks for each 
one, but now we also have a list of them in another place 
(ALLOWED_INFO_COMMANDS). The other confusing thing is that the INFO command 
does allow other keywords it just ignores them. In native redis "INFO foobar" 
does not give an error but just returns nothing. But by adding a 
RestrictedInputValuesParameterRequirements to INFO it makes me think that any 
other params would cause an error.
   
   OK-  SO I finally see your point about what INFO returns.  You are correct, 
there shouldn't be an error.  A false pass for an integration test was making 
me think that this was working correctly (that the error was being caught and 
only the error mesg was being returned to the end user). But that's not the 
case.  To your point, this should not be used for the info command then.  


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> create RestrictedInputValuesParameterRequirements in Redis Module
> -----------------------------------------------------------------
>
>                 Key: GEODE-8583
>                 URL: https://issues.apache.org/jira/browse/GEODE-8583
>             Project: Geode
>          Issue Type: Improvement
>            Reporter: John Hutchison
>            Priority: Minor
>              Labels: pull-request-available, redis
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to