Skip to content

Commit

Permalink
chore: log warnings for database tables api (#30410)
Browse files Browse the repository at this point in the history
  • Loading branch information
eschutho authored Sep 30, 2024
1 parent 9a5e8a4 commit 2e50167
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 17 deletions.
2 changes: 1 addition & 1 deletion superset/commands/database/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class DatabaseTestConnectionUnexpectedError(SupersetErrorsException):
message = _("Unexpected error occurred, please check your logs for details")


class DatabaseTablesUnexpectedError(Exception):
class DatabaseTablesUnexpectedError(CommandException):
status = 422
message = _("Unexpected error occurred, please check your logs for details")

Expand Down
2 changes: 1 addition & 1 deletion superset/commands/database/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def run(self) -> dict[str, Any]:
except SupersetException:
raise
except Exception as ex:
raise DatabaseTablesUnexpectedError(ex) from ex
raise DatabaseTablesUnexpectedError(str(ex)) from ex

def validate(self) -> None:
self._model = cast(Database, DatabaseDAO.find_by_id(self._db_id))
Expand Down
18 changes: 5 additions & 13 deletions superset/databases/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
DatabaseDeleteFailedError,
DatabaseInvalidError,
DatabaseNotFoundError,
DatabaseTablesUnexpectedError,
DatabaseUpdateFailedError,
InvalidParametersError,
)
Expand Down Expand Up @@ -131,7 +130,7 @@
requires_json,
statsd_metrics,
)
from superset.views.error_handling import json_error_response
from superset.views.error_handling import handle_api_exception, json_error_response
from superset.views.filters import BaseFilterRelatedUsers, FilterRelatedOwners

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -755,9 +754,9 @@ def schemas(self, pk: int, **kwargs: Any) -> FlaskResponse:

@expose("/<int:pk>/tables/")
@protect()
@safe
@rison(database_tables_query_schema)
@statsd_metrics
@handle_api_exception
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" f".tables",
log_to_statsd=False,
Expand Down Expand Up @@ -810,16 +809,9 @@ def tables(self, pk: int, **kwargs: Any) -> FlaskResponse:
catalog_name = kwargs["rison"].get("catalog_name")
schema_name = kwargs["rison"].get("schema_name", "")

try:
command = TablesDatabaseCommand(pk, catalog_name, schema_name, force)
payload = command.run()
return self.response(200, **payload)
except DatabaseNotFoundError:
return self.response_404()
except SupersetException as ex:
return self.response(ex.status, message=ex.message)
except DatabaseTablesUnexpectedError as ex:
return self.response_422(ex.message)
command = TablesDatabaseCommand(pk, catalog_name, schema_name, force)
payload = command.run()
return self.response(200, **payload)

@expose("/<int:pk>/table/<path:table_name>/<schema_name>/", methods=("GET",))
@protect()
Expand Down
13 changes: 11 additions & 2 deletions tests/integration_tests/databases/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2005,7 +2005,8 @@ def test_database_tables(self):
self.assertEqual(option["type"], "table")
self.assertTrue(option["value"] in schemas)

def test_database_tables_not_found(self):
@patch("superset.utils.log.logger")
def test_database_tables_not_found(self, logger_mock):
"""
Database API: Test database tables not found
"""
Expand All @@ -2014,6 +2015,9 @@ def test_database_tables_not_found(self):
uri = f"api/v1/database/{example_db.id}/tables/?q={prison.dumps({'schema_name': 'non_existent'})}"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)
logger_mock.warning.assert_called_once_with(
"Database not found.", exc_info=True
)

def test_database_tables_invalid_query(self):
"""
Expand All @@ -2026,8 +2030,12 @@ def test_database_tables_invalid_query(self):
)
self.assertEqual(rv.status_code, 400)

@patch("superset.utils.log.logger")
@mock.patch("superset.security.manager.SupersetSecurityManager.can_access_database")
def test_database_tables_unexpected_error(self, mock_can_access_database):
@mock.patch("superset.models.core.Database.get_all_table_names_in_schema")
def test_database_tables_unexpected_error(
self, mock_get_all_table_names_in_schema, mock_can_access_database, logger_mock
):
"""
Database API: Test database tables with unexpected error
"""
Expand All @@ -2039,6 +2047,7 @@ def test_database_tables_unexpected_error(self, mock_can_access_database):
f"api/v1/database/{database.id}/tables/?q={prison.dumps({'schema_name': 'main'})}"
)
self.assertEqual(rv.status_code, 422)
logger_mock.warning.assert_called_once_with("Test Error", exc_info=True)

def test_test_connection(self):
"""
Expand Down

0 comments on commit 2e50167

Please sign in to comment.