chore: enable ruff lint rule TRY201

When intercepting and re-raising an error in python, it's almost always better to simply `raise` over raising the exception alias ie: `raise ex`. The main reason is that the stack trace often (always?) gets truncated in the process, losing track of where the error occurred. Similarly, when intercepting and re-raising another exception, it's better to `raise AnotherException() from ex`, so I'm fixing and enforcing this as well. I found this while debugging and dealing with incomplete stack traces, and thought it be a good idea to fix and enforce using ruff's TRY201 and B904
apache · Jun 11, 2024 · f48bf74 · f48bf74
1 parent 024cfd8
commit f48bf74
Show file tree

Hide file tree

Showing 50 changed files with 152 additions and 141 deletions.
diff --git a/pyproject.toml b/pyproject.toml
@@ -435,7 +435,14 @@ target-version = "py310"
 # Enable Pyflakes (`F`) and a subset of the pycodestyle (`E`)  codes by default.
 # Unlike Flake8, Ruff doesn't enable pycodestyle warnings (`W`) or
 # McCabe complexity (`C901`) by default.
-select = ["E4", "E7", "E9", "F"]
+select = [
+    "B904",
+    "E4",
+    "E7",
+    "E9",
+    "F",
+    "TRY201",
+]
 ignore = []
 
 extend-select = ["I"]

diff --git a/superset/app.py b/superset/app.py
@@ -42,9 +42,9 @@ def create_app(superset_config_module: Optional[str] = None) -> Flask:
         return app
 
     # Make sure that bootstrap errors ALWAYS get logged
-    except Exception as ex:
+    except Exception:
         logger.exception("Failed to create app")
-        raise ex
+        raise
 
 
 class SupersetApp(Flask):

diff --git a/superset/charts/data/api.py b/superset/charts/data/api.py
@@ -446,5 +446,6 @@ def _create_query_context_from_form(
             return ChartDataQueryContextSchema().load(form_data)
         except KeyError as ex:
             raise ValidationError("Request is incorrect") from ex
-        except ValidationError as error:
-            raise error
+        except ValidationError:  # pylint: disable=try-except-raise
+            # Make sure to bubble this up
+            raise
diff --git a/superset/commands/chart/importers/dispatcher.py b/superset/commands/chart/importers/dispatcher.py
@@ -55,14 +55,14 @@ def run(self) -> None:
                 return
             except IncorrectVersionError:
                 logger.debug("File not handled by command, skipping")
-            except (CommandInvalidError, ValidationError) as exc:
+            except (CommandInvalidError, ValidationError):
                 # found right version, but file is invalid
                 logger.info("Command failed validation")
-                raise exc
-            except Exception as exc:
+                raise
+            except Exception:
                 # validation succeeded but something went wrong
                 logger.exception("Error running import command")
-                raise exc
+                raise
 
         raise CommandInvalidError("Could not find a valid command to import file")
 

diff --git a/superset/commands/dashboard/importers/dispatcher.py b/superset/commands/dashboard/importers/dispatcher.py
@@ -58,14 +58,14 @@ def run(self) -> None:
                 return
             except IncorrectVersionError:
                 logger.debug("File not handled by command, skipping")
-            except (CommandInvalidError, ValidationError) as exc:
+            except (CommandInvalidError, ValidationError):
                 # found right version, but file is invalid
                 logger.info("Command failed validation")
-                raise exc
-            except Exception as exc:
+                raise
+            except Exception:
                 # validation succeeded but something went wrong
                 logger.exception("Error running import command")
-                raise exc
+                raise
 
         raise CommandInvalidError("Could not find a valid command to import file")
 

diff --git a/superset/commands/database/create.py b/superset/commands/database/create.py
@@ -68,7 +68,7 @@ def run(self) -> Model:
                 engine=self._properties.get("sqlalchemy_uri", "").split(":")[0],
             )
             # So we can show the original message
-            raise ex
+            raise
         except Exception as ex:
             event_logger.log_with_context(
                 action=f"db_creation_failed.{ex.__class__.__name__}",
@@ -141,7 +141,7 @@ def run(self) -> Model:
                 engine=self._properties.get("sqlalchemy_uri", "").split(":")[0],
             )
             # So we can show the original message
-            raise ex
+            raise
         except (
             DAOCreateFailedError,
             DatabaseInvalidError,

diff --git a/superset/commands/database/importers/dispatcher.py b/superset/commands/database/importers/dispatcher.py
@@ -53,14 +53,14 @@ def run(self) -> None:
                 return
             except IncorrectVersionError:
                 logger.debug("File not handled by command, skipping")
-            except (CommandInvalidError, ValidationError) as exc:
+            except (CommandInvalidError, ValidationError):
                 # found right version, but file is invalid
                 logger.info("Command failed validation")
-                raise exc
-            except Exception as exc:
+                raise
+            except Exception:
                 # validation succeeded but something went wrong
                 logger.exception("Error running import command")
-                raise exc
+                raise
 
         raise CommandInvalidError("Could not find a valid command to import file")
 

diff --git a/superset/commands/database/ssh_tunnel/create.py b/superset/commands/database/ssh_tunnel/create.py
@@ -51,8 +51,9 @@ def run(self) -> Model:
             return ssh_tunnel
         except DAOCreateFailedError as ex:
             raise SSHTunnelCreateFailedError() from ex
-        except SSHTunnelInvalidError as ex:
-            raise ex
+        except SSHTunnelInvalidError:  # pylint: disable=try-except-raise
+            # Make sure to bubble this up
+            raise
 
     def validate(self) -> None:
         # TODO(hughhh): check to make sure the server port is not localhost

diff --git a/superset/commands/database/tables.py b/superset/commands/database/tables.py
@@ -127,8 +127,8 @@ def run(self) -> dict[str, Any]:
 
             payload = {"count": len(tables) + len(views), "result": options}
             return payload
-        except SupersetException as ex:
-            raise ex
+        except SupersetException:
+            raise
         except Exception as ex:
             raise DatabaseTablesUnexpectedError(ex) from ex
 

diff --git a/superset/commands/database/test_connection.py b/superset/commands/database/test_connection.py
@@ -212,7 +212,7 @@ def ping(engine: Engine) -> bool:
                 engine=database.db_engine_spec.__name__,
             )
             # bubble up the exception to return a 408
-            raise ex
+            raise
         except SSHTunnelingNotEnabledError as ex:
             event_logger.log_with_context(
                 action=get_log_connection_action(
@@ -221,7 +221,7 @@ def ping(engine: Engine) -> bool:
                 engine=database.db_engine_spec.__name__,
             )
             # bubble up the exception to return a 400
-            raise ex
+            raise
         except Exception as ex:
             event_logger.log_with_context(
                 action=get_log_connection_action(

diff --git a/superset/commands/database/update.py b/superset/commands/database/update.py
@@ -85,9 +85,9 @@ def run(self) -> Model:
             database.set_sqlalchemy_uri(database.sqlalchemy_uri)
             ssh_tunnel = self._handle_ssh_tunnel(database)
             self._refresh_catalogs(database, original_database_name, ssh_tunnel)
-        except SSHTunnelError as ex:
+        except SSHTunnelError:  # pylint: disable=try-except-raise
             # allow exception to bubble for debugbing information
-            raise ex
+            raise
         except (DAOUpdateFailedError, DAOCreateFailedError) as ex:
             raise DatabaseUpdateFailedError() from ex
 

diff --git a/superset/commands/dataset/importers/dispatcher.py b/superset/commands/dataset/importers/dispatcher.py
@@ -58,14 +58,14 @@ def run(self) -> None:
                 return
             except IncorrectVersionError:
                 logger.debug("File not handled by command, skipping")
-            except (CommandInvalidError, ValidationError) as exc:
+            except (CommandInvalidError, ValidationError):
                 # found right version, but file is invalid
                 logger.info("Command failed validation")
-                raise exc
-            except Exception as exc:
+                raise
+            except Exception:
                 # validation succeeded but something went wrong
                 logger.exception("Error running import command")
-                raise exc
+                raise
 
         raise CommandInvalidError("Could not find a valid command to import file")
 

diff --git a/superset/commands/importers/v1/__init__.py b/superset/commands/importers/v1/__init__.py
@@ -74,9 +74,9 @@ def run(self) -> None:
         try:
             self._import(self._configs, self.overwrite)
             db.session.commit()
-        except CommandException as ex:
+        except CommandException:
             db.session.rollback()
-            raise ex
+            raise
         except Exception as ex:
             db.session.rollback()
             raise self.import_error() from ex

diff --git a/superset/commands/importers/v1/utils.py b/superset/commands/importers/v1/utils.py
@@ -75,7 +75,7 @@ def load_metadata(contents: dict[str, str]) -> dict[str, str]:
 
         # otherwise we raise the validation error
         ex.messages = {METADATA_FILE_NAME: ex.messages}
-        raise ex
+        raise
 
     return metadata
 

diff --git a/superset/commands/query/importers/dispatcher.py b/superset/commands/query/importers/dispatcher.py
@@ -55,10 +55,10 @@ def run(self) -> None:
                 return
             except IncorrectVersionError:
                 logger.debug("File not handled by command, skipping")
-            except (CommandInvalidError, ValidationError) as exc:
+            except (CommandInvalidError, ValidationError):
                 # found right version, but file is invalid
                 logger.exception("Error running import command")
-                raise exc
+                raise
 
         raise CommandInvalidError("Could not find a valid command to import file")
 

diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py
@@ -599,7 +599,7 @@ def next(self) -> None:
                     self.update_report_schedule_and_log(
                         ReportState.ERROR, error_message=second_error_message
                     )
-            raise first_ex
+            raise
 
 
 class ReportWorkingState(BaseReportState):
@@ -662,7 +662,7 @@ def next(self) -> None:
                     ReportState.ERROR,
                     error_message=REPORT_SCHEDULE_ERROR_NOTIFICATION_MARKER,
                 )
-                raise ex
+                raise
 
         try:
             self.send()
@@ -737,8 +737,8 @@ def run(self) -> None:
                 ReportScheduleStateMachine(
                     self._execution_id, self._model, self._scheduled_dttm
                 ).run()
-        except CommandException as ex:
-            raise ex
+        except CommandException:
+            raise
         except Exception as ex:
             raise ReportScheduleUnexpectedError(str(ex)) from ex
 

diff --git a/superset/commands/security/create.py b/superset/commands/security/create.py
@@ -42,7 +42,7 @@ def run(self) -> Any:
             return RLSDAO.create(attributes=self._properties)
         except DAOCreateFailedError as ex:
             logger.exception(ex.exception)
-            raise ex
+            raise
 
     def validate(self) -> None:
         roles = populate_roles(self._roles)

diff --git a/superset/commands/security/update.py b/superset/commands/security/update.py
@@ -47,7 +47,7 @@ def run(self) -> Any:
             rule = RLSDAO.update(self._model, self._properties)
         except DAOUpdateFailedError as ex:
             logger.exception(ex.exception)
-            raise ex
+            raise
 
         return rule
 

diff --git a/superset/commands/sql_lab/execute.py b/superset/commands/sql_lab/execute.py
@@ -115,10 +115,10 @@ def run(  # pylint: disable=too-many-statements,useless-suppression
                 "status": status,
                 "payload": self._execution_context_convertor.serialize_payload(),
             }
-        except (SupersetErrorException, SupersetErrorsException) as ex:
+        except (SupersetErrorException, SupersetErrorsException):
             # to make sure we raising the original
             # SupersetErrorsException || SupersetErrorsException
-            raise ex
+            raise
         except Exception as ex:
             raise SqlLabException(self._execution_context, exception=ex) from ex
 
@@ -158,9 +158,9 @@ def _run_sql_json_exec_from_scratch(self) -> SqlJsonExecutionStatus:
             return self._sql_json_executor.execute(
                 self._execution_context, rendered_query, self._log_params
             )
-        except Exception as ex:
+        except Exception:
             self._query_dao.update(query, {"status": QueryStatus.FAILED})
-            raise ex
+            raise
 
     def _get_the_query_db(self) -> Database:
         mydb: Any = self._database_dao.find_by_id(self._execution_context.database_id)

diff --git a/superset/common/query_object.py b/superset/common/query_object.py
@@ -276,7 +276,7 @@ def validate(
             return None
         except QueryObjectValidationError as ex:
             if raise_exceptions:
-                raise ex
+                raise
             return ex
 
     def _validate_no_have_duplicate_labels(self) -> None:

diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
@@ -1727,11 +1727,11 @@ def assign_column_label(df: pd.DataFrame) -> pd.DataFrame | None:
                 self.schema or None,
                 mutator=assign_column_label,
             )
-        except (SupersetErrorException, SupersetErrorsException) as ex:
+        except (SupersetErrorException, SupersetErrorsException):
             # SupersetError(s) exception should not be captured; instead, they should
             # bubble up to the Flask error handler so they are returned as proper SIP-40
             # errors. This is particularly important for database OAuth2, see SIP-85.
-            raise ex
+            raise
         except Exception as ex:  # pylint: disable=broad-except
             # TODO (betodealmeida): review exception handling while querying the external
             # database. Ideally we'd expect and handle external database error, but

diff --git a/superset/databases/utils.py b/superset/databases/utils.py
@@ -123,8 +123,8 @@ def make_url_safe(raw_url: str | URL) -> URL:
         url = raw_url.strip()
         try:
             return make_url(url)  # noqa
-        except Exception:
-            raise DatabaseInvalidError()  # pylint: disable=raise-missing-from
+        except Exception as ex:
+            raise DatabaseInvalidError() from ex
 
     else:
         return raw_url
diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
@@ -2001,7 +2001,7 @@ def get_extra_params(database: Database) -> dict[str, Any]:
                 extra = json.loads(database.extra)
             except json.JSONDecodeError as ex:
                 logger.error(ex, exc_info=True)
-                raise ex
+                raise
         return extra
 
     @staticmethod
@@ -2022,7 +2022,7 @@ def update_params_from_encrypted_extra(  # pylint: disable=invalid-name
             params.update(encrypted_extra)
         except json.JSONDecodeError as ex:
             logger.error(ex, exc_info=True)
-            raise ex
+            raise
 
     @classmethod
     def is_readonly_query(cls, parsed_query: ParsedQuery) -> bool:

diff --git a/superset/db_engine_specs/impala.py b/superset/db_engine_specs/impala.py
@@ -104,7 +104,7 @@ def execute(
         try:
             cursor.execute_async(query)
         except Exception as ex:
-            raise cls.get_dbapi_mapped_exception(ex)
+            raise cls.get_dbapi_mapped_exception(ex) from ex
 
     @classmethod
     def handle_cursor(cls, cursor: Any, query: Query) -> None:

diff --git a/superset/db_engine_specs/ocient.py b/superset/db_engine_specs/ocient.py
@@ -315,12 +315,12 @@ def fetch_data(
     ) -> list[tuple[Any, ...]]:
         try:
             rows: list[tuple[Any, ...]] = super().fetch_data(cursor, limit)
-        except Exception as exception:
+        except Exception:
             with OcientEngineSpec.query_id_mapping_lock:
                 del OcientEngineSpec.query_id_mapping[
                     getattr(cursor, "superset_query_id")
                 ]
-            raise exception
+            raise
 
         # TODO: Unsure if we need to verify that we are receiving rows:
         if len(rows) > 0 and type(rows[0]).__name__ == "Row":

diff --git a/superset/db_engine_specs/snowflake.py b/superset/db_engine_specs/snowflake.py
@@ -368,7 +368,7 @@ def update_params_from_encrypted_extra(
             encrypted_extra = json.loads(database.encrypted_extra)
         except json.JSONDecodeError as ex:
             logger.error(ex, exc_info=True)
-            raise ex
+            raise
         auth_method = encrypted_extra.get("auth_method", None)
         auth_params = encrypted_extra.get("auth_params", {})
         if not auth_method:

diff --git a/superset/db_engine_specs/trino.py b/superset/db_engine_specs/trino.py
@@ -342,7 +342,7 @@ def update_params_from_encrypted_extra(
             connect_args["auth"] = trino_auth(**auth_params)
         except json.JSONDecodeError as ex:
             logger.error(ex, exc_info=True)
-            raise ex
+            raise
 
     @classmethod
     def get_dbapi_exception_mapping(cls) -> dict[type[Exception], type[Exception]]:

diff --git a/...grations/versions/2018-07-05_15-19_3dda56f1c4c6_migrate_num_period_compare_and_period_.py b/...grations/versions/2018-07-05_15-19_3dda56f1c4c6_migrate_num_period_compare_and_period_.py
@@ -118,13 +118,13 @@ def compute_time_compare(granularity, periods):
 
     try:
         obj = isodate.parse_duration(granularity) * periods
-    except isodate.isoerror.ISO8601Error:
+    except isodate.isoerror.ISO8601Error as ex:
         # if parse_human_timedelta can parse it, return it directly
         delta = f"{periods} {granularity}{'s' if periods > 1 else ''}"
         obj = parse_human_timedelta(delta)
         if obj:
             return delta
-        raise Exception(f"Unable to parse: {granularity}")
+        raise Exception(f"Unable to parse: {granularity}") from ex
 
     if isinstance(obj, isodate.duration.Duration):
         return isodate_duration_to_string(obj)