[GitHub] [incubator-superset] kkucharc commented on pull request #10942: fix: setting specific exceptions common/query_context.py

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

kkucharc commented on pull request #10942:
URL: https://github.com/apache/incubator-superset/pull/10942#issuecomment-694838410


   @mistercrunch I totally agree with having broad-except, especially when we have as you said various integrations. My suggestions in the PR comes from my intuition - I can imagine there can be some RuntimeExceptions or even something more complicated memory issues, which can be neglected by broad-exception.
   
   If I may share my opinion about adding `broad-except` to global disabled rules, I would disagree, because of two reasons:
   1. probably if project will achieve big amount of broad exceptions it will be difficult and painful to come back,
   2. I'm afraid of over-use broad exception, because it's much easier to react to _any_ bad thing that may happen than think deeper how to address each error one by one. I guess also it may extend reviewing process.
   But of course you know wider context of the subject and requirements, so I am totally open to adding global `broad-except` and/or for further discussion 👍 
   
   Also if it comes to the current PR I am open to closing this PR if it may cause more harm than good. I fully agree that it might be worth to take care about all broad exceptions to get the most of them.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org