nchammas commented on PR #58282: URL: https://github.com/apache/spark/pull/58282#issuecomment-5500176000
> Can you explain why simply allowing an exception to not use a subclass if none apply, is bad? It's a fine idea. My concern is about the mix of patterns we have across the project for structuring error conditions. I'm trying to constrain them so things stay as consistent as possible. I don't want to add a new pattern without having a plan to clean up some of the old ones; we already have conflicting conventions in use today, and things are incrementally getting worse. How about this updated proposal? When creating an error condition with an optional message (like a hint, suggestion, or similar): 1. If the condition is a top-level condition, make the optional message a sub-condition. e.g. `CONDITION` and `CONDITION.WITH_HINT`. 2. If the condition is a sub-condition, make the optional message a sibling condition. e.g. `CONDITION.SUB` and `CONDITION.SUB_WITH_HINT`. 3. Don't include `_WITHOUT_HINT` (or similar) in the name of any error condition. 4. Don't pass in empty strings to optional message parameters like hints or suggestions. Instead, create a sub-condition that always includes that message. If we agree on this approach, then I would be happy to see this PR merged. We can then update the error condition README with this guidance and, perhaps as a follow-up to SPARK-58824, update existing conditions to follow it. -- 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]
