imply-cheddar commented on PR #14004: URL: https://github.com/apache/druid/pull/14004#issuecomment-1586644312
@abhishekagarwal87 On your suggestion about encouraging an action be included. You gave a great example that explains what this PR is all about. Specifically, think of this error message ``` org.apache.druid.java.util.common.ISE: No default server found! ``` If someone said that it was "Persona.USER", I would hope that the reviewer would be like, "uhhh, why will a user know what a default server is? Your message is not well aligned with your persona" and ask for the message to be updated. If I were to adjust that exception to this new model and not change the message, I would likely make it `Persona.DEVELOPER` because it's so obtuse, only a developer would be able to actually make sense of what it means. Of course, the exception is actually happening at a point where there is some behavior that is meaningful for the end user. We could adjust the message to be an ADMIN persona message and say ``` Router unable to find a broker, check that brokers are up and active ``` Or a USER persona message and say ``` Cannot find a queryable server, contact your cluster administrator to validate that all services are operational ``` Both of these include an action, like you suggest, but that action becomes apparent and clear once we actually identify the persona. My *hope* is that by identifying the persona, reviewers will start to validate that the exception messages align with the intended persona and we all police each other for creating good error messages. I.e. I'm hoping that just identifying the persona is enough to help us do better. Alternatively, for USER-facing messages, I could add the requirement for yet-another field which is "potential actions" (or something similar) which should be populated with a message describing actions that can be taken. I'd personally like to keep it to just a message and code-review based policing. But I'm also willing to add the field if I'm in the minority... Tagging other people whose name also appears in the conversation part of this PR to see if there are opinions one way or the other: @gianm @kfaraz @clintropolis -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
