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: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
