dpgaspar edited a comment on pull request #11973: URL: https://github.com/apache/incubator-superset/pull/11973#issuecomment-741715540
> It feels pretty harsh to catch `Exception`, I wonder if there is some slightly more specific exception (or small set of exceptions) that could be used to catch the majority of these. Another option could be adding db specific exceptions to the db engine spec and then checking for those. Something like > > ```python > def get_dbapi_exceptions(): Tuple[Exception, ...]: > from pydruid import CustomPydryidException > return (CustomPydryidException, ) > ``` I really don't like introducing broad exceptions, but `dbapi` exception's are raising custom exceptions from `Exception`. Yet created this related PR on pydruid to set SQLAlchemy `visit_*` methods raise from `CompileError`: https://github.com/druid-io/pydruid/pull/243 I think is what we should expect here. But other drivers will raise childs of `Exception`. Introducing all these imports inside superset will increase complexity and maintainability, IMO better to live with the broad except ---------------------------------------------------------------- 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]
