You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/08/22 08:35:45 UTC

[GitHub] [helix] pkuwm edited a comment on pull request #1237: Add error message of instance check to stoppable API response

pkuwm edited a comment on pull request #1237:
URL: https://github.com/apache/helix/pull/1237#issuecomment-678613525


   > Error message for server side is clear to find out the fail reason. The more important is let user to understand what is the real fail reason. Do we have that if persist assignment turned off, at least, we gave user info EV and IS does not match.
   
   @dasahcc I think it is more appropriate to put this config check in `INVALID_CONFIG` health check, which is the first check in health status check. If configs are invalid, we don't need to do remaining checks like INSTANCE_NOT_STABLE.
   
   The response should be like this 
   ```
   curl -XPOST http://localhost:8100/admin/v2/namespaces/namespacce/clusters/cluster/instances\?command\=stoppable -d ' {"instances": ["instance"], "selection_base": "zone_based", "max_instance": "2", "customized_values": "{}"}' -H "Content-Type: application/json"
   {
     "instance_stoppable_parallel" : [ ],
     "instance_not_stoppable_with_reasons" : {
       "instance" : [ "HELIX:INVALID_CONFIG" ]
     }
   }
   ```
   
   I actually would like to add more info for users to know why. But with current implementation, it is not easy/straightforward to add the message. Maybe ` "HELIX:INVALID_CONFIG:Cluster config PERSIST_INTERMEDIATE_ASSIGNMENT is not turned on"
   
   I think the better one would be a map response for each instance: 
   ```
   {
     "instance_stoppable_parallel" : [ ],
     "instance_not_stoppable_with_reasons" : {
       "instance" : "hostname",
       "health-check": [ "HELIX:INVALID_CONFIG" ],
       "message": "Cluster config PERSIST_INTERMEDIATE_ASSIGNMENT is not turned on"
     }
   }
   ```


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org