Skip to content

Commit

Permalink
chore: enable ruff lint rule TRY201 and B904 to improve raise stack…
Browse files Browse the repository at this point in the history
… traces (apache#29166)
  • Loading branch information
mistercrunch authored Jun 12, 2024
1 parent 5167d20 commit 4bb2e2f
Show file tree
Hide file tree
Showing 50 changed files with 152 additions and 141 deletions.
9 changes: 8 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
4 changes: 2 additions & 2 deletions superset/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
5 changes: 3 additions & 2 deletions superset/charts/data/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 4 additions & 4 deletions superset/commands/chart/importers/dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
8 changes: 4 additions & 4 deletions superset/commands/dashboard/importers/dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
4 changes: 2 additions & 2 deletions superset/commands/database/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__}",
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions superset/commands/database/importers/dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
5 changes: 3 additions & 2 deletions superset/commands/database/ssh_tunnel/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions superset/commands/database/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions superset/commands/database/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions superset/commands/database/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions superset/commands/dataset/importers/dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
4 changes: 2 additions & 2 deletions superset/commands/importers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion superset/commands/importers/v1/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions superset/commands/query/importers/dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
8 changes: 4 additions & 4 deletions superset/commands/report/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -662,7 +662,7 @@ def next(self) -> None:
ReportState.ERROR,
error_message=REPORT_SCHEDULE_ERROR_NOTIFICATION_MARKER,
)
raise ex
raise

try:
self.send()
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion superset/commands/security/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion superset/commands/security/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions superset/commands/sql_lab/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion superset/common/query_object.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions superset/databases/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions superset/db_engine_specs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion superset/db_engine_specs/impala.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions superset/db_engine_specs/ocient.py
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down
2 changes: 1 addition & 1 deletion superset/db_engine_specs/snowflake.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion superset/db_engine_specs/trino.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 4bb2e2f

Please sign in to comment.