Skip to content

Commit

Permalink
add logging on successful data uploads (#17065)
Browse files Browse the repository at this point in the history
  • Loading branch information
eschutho authored Oct 11, 2021
1 parent c734035 commit c2e1ab6
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 14 deletions.
22 changes: 19 additions & 3 deletions superset/views/database/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from superset.connectors.sqla.models import SqlaTable
from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.exceptions import CertificateException
from superset.extensions import event_logger
from superset.sql_parse import Table
from superset.typing import FlaskResponse
from superset.utils import core as utils
Expand Down Expand Up @@ -252,7 +253,12 @@ def form_post(self, form: CsvToDatabaseForm) -> Response:
db_name=sqla_table.database.database_name,
)
flash(message, "info")
stats_logger.incr("successful_csv_upload")
event_logger.log_with_context(
action="successful_csv_upload",
database=form.con.data.name,
schema=form.schema.data,
table=form.name.data,
)
return redirect("/tablemodelview/list/")


Expand Down Expand Up @@ -393,7 +399,12 @@ def form_post(self, form: ExcelToDatabaseForm) -> Response:
db_name=sqla_table.database.database_name,
)
flash(message, "info")
stats_logger.incr("successful_excel_upload")
event_logger.log_with_context(
action="successful_excel_upload",
database=form.con.data.name,
schema=form.schema.data,
table=form.name.data,
)
return redirect("/tablemodelview/list/")


Expand Down Expand Up @@ -540,5 +551,10 @@ def form_post( # pylint: disable=too-many-locals
db_name=sqla_table.database.database_name,
)
flash(message, "info")
stats_logger.incr("successful_columnar_upload")
event_logger.log_with_context(
action="successful_columnar_upload",
database=form.con.data.name,
schema=form.schema.data,
table=form.name.data,
)
return redirect("/tablemodelview/list/")
67 changes: 56 additions & 11 deletions tests/integration_tests/csv_upload_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,15 @@ def mock_upload_to_s3(filename: str, upload_prefix: str, table: Table) -> str:
return dest_dir


@pytest.mark.usefixtures("setup_csv_upload")
@pytest.mark.usefixtures("create_csv_files")
@mock.patch(
"superset.models.core.config",
{**app.config, "ALLOWED_USER_CSV_SCHEMA_FUNC": lambda d, u: ["admin_database"]},
)
@mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3)
def test_import_csv_enforced_schema(setup_csv_upload, create_csv_files):
@mock.patch("superset.views.database.views.event_logger.log_with_context")
def test_import_csv_enforced_schema(mock_event_logger):
if utils.backend() == "sqlite":
pytest.skip("Sqlite doesn't support schema / database creation")

Expand All @@ -218,6 +221,12 @@ def test_import_csv_enforced_schema(setup_csv_upload, create_csv_files):
extra={"schema": "admin_database", "if_exists": "replace"},
)
assert success_msg in resp
mock_event_logger.assert_called_with(
action="successful_csv_upload",
database=get_upload_db().name,
schema="admin_database",
table=CSV_UPLOAD_TABLE_W_SCHEMA,
)

engine = get_upload_db().get_sqla_engine()
data = engine.execute(
Expand Down Expand Up @@ -259,12 +268,17 @@ def test_import_csv_explore_database(setup_csv_upload, create_csv_files):
assert table.database_id == utils.get_example_database().id


@pytest.mark.usefixtures("setup_csv_upload")
@pytest.mark.usefixtures("create_csv_files")
@mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3)
def test_import_csv(setup_csv_upload, create_csv_files):
@mock.patch("superset.views.database.views.event_logger.log_with_context")
def test_import_csv(mock_event_logger):
success_msg_f1 = (
f'CSV file "{CSV_FILENAME1}" uploaded to table "{CSV_UPLOAD_TABLE}"'
)

test_db = get_upload_db()

# initial upload with fail mode
resp = upload_csv(CSV_FILENAME1, CSV_UPLOAD_TABLE)
assert success_msg_f1 in resp
Expand All @@ -282,6 +296,12 @@ def test_import_csv(setup_csv_upload, create_csv_files):
CSV_FILENAME1, CSV_UPLOAD_TABLE, extra={"if_exists": "append"}
)
assert success_msg_f1 in resp
mock_event_logger.assert_called_with(
action="successful_csv_upload",
database=test_db.name,
schema=None,
table=CSV_UPLOAD_TABLE,
)

# upload again with replace mode and specific columns
resp = upload_csv(
Expand Down Expand Up @@ -324,7 +344,7 @@ def test_import_csv(setup_csv_upload, create_csv_files):
extra={"null_values": '["", "john"]', "if_exists": "replace"},
)
# make sure that john and empty string are replaced with None
engine = get_upload_db().get_sqla_engine()
engine = test_db.get_sqla_engine()
data = engine.execute(f"SELECT * from {CSV_UPLOAD_TABLE}").fetchall()
assert data == [(None, 1, "x"), ("paul", 2, None)]

Expand All @@ -335,18 +355,29 @@ def test_import_csv(setup_csv_upload, create_csv_files):
assert data == [("john", 1, "x"), ("paul", 2, None)]


@pytest.mark.usefixtures("setup_csv_upload")
@pytest.mark.usefixtures("create_excel_files")
@mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3)
def test_import_excel(setup_csv_upload, create_excel_files):
@mock.patch("superset.views.database.views.event_logger.log_with_context")
def test_import_excel(mock_event_logger):
if utils.backend() == "hive":
pytest.skip("Hive doesn't excel upload.")

test_db = get_upload_db()

success_msg = (
f'Excel file "{EXCEL_FILENAME}" uploaded to table "{EXCEL_UPLOAD_TABLE}"'
)

# initial upload with fail mode
resp = upload_excel(EXCEL_FILENAME, EXCEL_UPLOAD_TABLE)
assert success_msg in resp
mock_event_logger.assert_called_with(
action="successful_excel_upload",
database=test_db.name,
schema=None,
table=EXCEL_UPLOAD_TABLE,
)

# upload again with fail mode; should fail
fail_msg = f'Unable to upload Excel file "{EXCEL_FILENAME}" to table "{EXCEL_UPLOAD_TABLE}"'
Expand All @@ -365,22 +396,32 @@ def test_import_excel(setup_csv_upload, create_excel_files):
EXCEL_FILENAME, EXCEL_UPLOAD_TABLE, extra={"if_exists": "replace"}
)
assert success_msg in resp
mock_event_logger.assert_called_with(
action="successful_excel_upload",
database=test_db.name,
schema=None,
table=EXCEL_UPLOAD_TABLE,
)

# make sure that john and empty string are replaced with None
data = (
get_upload_db()
.get_sqla_engine()
test_db.get_sqla_engine()
.execute(f"SELECT * from {EXCEL_UPLOAD_TABLE}")
.fetchall()
)
assert data == [(0, "john", 1), (1, "paul", 2)]


@pytest.mark.usefixtures("setup_csv_upload")
@pytest.mark.usefixtures("create_columnar_files")
@mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3)
def test_import_parquet(setup_csv_upload, create_columnar_files):
@mock.patch("superset.views.database.views.event_logger.log_with_context")
def test_import_parquet(mock_event_logger):
if utils.backend() == "hive":
pytest.skip("Hive doesn't allow parquet upload.")

test_db = get_upload_db()

success_msg_f1 = f'Columnar file "[\'{PARQUET_FILENAME1}\']" uploaded to table "{PARQUET_UPLOAD_TABLE}"'

# initial upload with fail mode
Expand All @@ -398,6 +439,12 @@ def test_import_parquet(setup_csv_upload, create_columnar_files):
PARQUET_FILENAME1, PARQUET_UPLOAD_TABLE, extra={"if_exists": "append"}
)
assert success_msg_f1 in resp
mock_event_logger.assert_called_with(
action="successful_columnar_upload",
database=test_db.name,
schema=None,
table=PARQUET_UPLOAD_TABLE,
)

# upload again with replace mode and specific columns
resp = upload_columnar(
Expand All @@ -418,8 +465,7 @@ def test_import_parquet(setup_csv_upload, create_columnar_files):
assert success_msg_f1 in resp

data = (
get_upload_db()
.get_sqla_engine()
test_db.get_sqla_engine()
.execute(f"SELECT * from {PARQUET_UPLOAD_TABLE} ORDER BY b")
.fetchall()
)
Expand All @@ -433,8 +479,7 @@ def test_import_parquet(setup_csv_upload, create_columnar_files):
assert success_msg_f2 in resp

data = (
get_upload_db()
.get_sqla_engine()
test_db.get_sqla_engine()
.execute(f"SELECT * from {PARQUET_UPLOAD_TABLE} ORDER BY b")
.fetchall()
)
Expand Down

0 comments on commit c2e1ab6

Please sign in to comment.