[GitHub] [flink] bowenli86 edited a comment on issue #10053: [FLINK-14578][table] load/unloadModule() should throw RuntimeException rather than checked exception

[GitBox <gi...@apache.org>]

bowenli86 edited a comment on issue #10053: [FLINK-14578][table] load/unloadModule() should throw RuntimeException rather than checked exception
URL: https://github.com/apache/flink/pull/10053#issuecomment-548984963
 
 
   > Thanks for the contribution. The diff itself looks fine. However, wouldn't it be better if we change existing exception classes (such as ModuleNotFoundException) to runtime?
   > 
   > The downside of having a general exception (such as ValidationException) is the difficult for an exception handler to differentiate the root cause. (Otherwise, one would have to parse the exception msg.)
   > 
   > Thus, I prefer retaining the existing exception classes but changing them to runtime.
   
   I'm not sure if catching the unchecked exception here is optimal for users as they are not declared in API signature and users aren't sure what unchecked exceptions will be thrown unless looking at implementations. Besides, loading/unloading module will most likely happen via config or interactive programming, in which case the error message matters more. If users ran into this via a table API program, their program has bugs and they'd better fix it rather than catching the exceptions. Thus I think throwing ValidationException is fine, and it aligns better with other parts of Flink SQL.
   
   Though it seems a bit unnecessary given the above use case explained, I also don't have strong opinions of making ModuleNotExistException/ModuleAlreadyExistException RuntimeException or extend ValidationException.

----------------------------------------------------------------
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


With regards,
Apache Git Services