-
Notifications
You must be signed in to change notification settings - Fork 13.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(reports): xlsx attachments #19810
base: master
Are you sure you want to change the base?
Changes from all commits
11eb929
d8559ef
da40f7d
150a936
440141c
92fdd21
b0c155f
e042486
1270180
f578550
2147246
3edb8d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -17,6 +17,7 @@ | |||||||||||
import json | ||||||||||||
import logging | ||||||||||||
from datetime import datetime, timedelta | ||||||||||||
from io import BytesIO | ||||||||||||
from typing import Any, Optional, Union | ||||||||||||
from uuid import UUID | ||||||||||||
|
||||||||||||
|
@@ -158,6 +159,7 @@ def _get_url( | |||||||||||
if self._report_schedule.chart: | ||||||||||||
if result_format in { | ||||||||||||
ChartDataResultFormat.CSV, | ||||||||||||
ChartDataResultFormat.XLSX, | ||||||||||||
ChartDataResultFormat.JSON, | ||||||||||||
}: | ||||||||||||
return get_url_path( | ||||||||||||
|
@@ -330,13 +332,25 @@ def _get_log_data(self) -> HeaderDataType: | |||||||||||
} | ||||||||||||
return log_data | ||||||||||||
|
||||||||||||
def _get_xlsx_data(self) -> bytes: | ||||||||||||
csv_data = self._get_csv_data() | ||||||||||||
|
||||||||||||
df = pd.read_csv(BytesIO(csv_data)) | ||||||||||||
bio = BytesIO() | ||||||||||||
|
||||||||||||
# pylint: disable=abstract-class-instantiated | ||||||||||||
with pd.ExcelWriter(bio, engine="openpyxl") as writer: | ||||||||||||
df.to_excel(writer, index=False) | ||||||||||||
cemremengu marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
|
||||||||||||
return bio.getvalue() | ||||||||||||
|
||||||||||||
def _get_notification_content(self) -> NotificationContent: | ||||||||||||
""" | ||||||||||||
Gets a notification content, this is composed by a title and a screenshot | ||||||||||||
|
||||||||||||
:raises: ReportScheduleScreenshotFailedError | ||||||||||||
""" | ||||||||||||
csv_data = None | ||||||||||||
data = None | ||||||||||||
embedded_data = None | ||||||||||||
error_text = None | ||||||||||||
screenshot_data = [] | ||||||||||||
|
@@ -352,11 +366,18 @@ def _get_notification_content(self) -> NotificationContent: | |||||||||||
error_text = "Unexpected missing screenshot" | ||||||||||||
elif ( | ||||||||||||
self._report_schedule.chart | ||||||||||||
and self._report_schedule.report_format == ReportDataFormat.DATA | ||||||||||||
and self._report_schedule.report_format == ReportDataFormat.CSV | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add if self._report_schedule.report_format in ReportDataFormat.table_like(): For @classmethod
def table_like(cls) -> Set["ReportDataFormat"]:
return {cls.CSV} | {cls.XLSX} |
||||||||||||
): | ||||||||||||
csv_data = self._get_csv_data() | ||||||||||||
if not csv_data: | ||||||||||||
data = self._get_csv_data() | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to create one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @EugeneTorap If you go this route (use superset/superset/charts/post_processing.py Lines 369 to 373 in ddd8d17
|
||||||||||||
if not data: | ||||||||||||
error_text = "Unexpected missing csv file" | ||||||||||||
elif ( | ||||||||||||
self._report_schedule.chart | ||||||||||||
and self._report_schedule.report_format == ReportDataFormat.XLSX | ||||||||||||
): | ||||||||||||
data = self._get_xlsx_data() | ||||||||||||
if not data: | ||||||||||||
error_text = "Unexpected missing csv data for xlsx" | ||||||||||||
if error_text: | ||||||||||||
return NotificationContent( | ||||||||||||
name=self._report_schedule.name, | ||||||||||||
|
@@ -386,7 +407,8 @@ def _get_notification_content(self) -> NotificationContent: | |||||||||||
url=url, | ||||||||||||
screenshots=screenshot_data, | ||||||||||||
description=self._report_schedule.description, | ||||||||||||
csv=csv_data, | ||||||||||||
data=data, | ||||||||||||
data_format=self._report_schedule.report_format, | ||||||||||||
embedded_data=embedded_data, | ||||||||||||
header_data=header_data, | ||||||||||||
) | ||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -205,7 +205,21 @@ def create_report_email_chart_with_csv(): | |
report_schedule = create_report_notification( | ||
email_target="[email protected]", | ||
chart=chart, | ||
report_format=ReportDataFormat.DATA, | ||
report_format=ReportDataFormat.CSV, | ||
) | ||
yield report_schedule | ||
cleanup_report_schedule(report_schedule) | ||
|
||
|
||
@pytest.fixture() | ||
def create_report_email_chart_with_xlsx(): | ||
with app.app_context(): | ||
chart = db.session.query(Slice).first() | ||
chart.query_context = '{"mock": "query_context"}' | ||
report_schedule = create_report_notification( | ||
email_target="[email protected]", | ||
chart=chart, | ||
report_format=ReportDataFormat.XLSX, | ||
) | ||
yield report_schedule | ||
cleanup_report_schedule(report_schedule) | ||
|
@@ -233,13 +247,28 @@ def create_report_email_chart_with_csv_no_query_context(): | |
report_schedule = create_report_notification( | ||
email_target="[email protected]", | ||
chart=chart, | ||
report_format=ReportDataFormat.DATA, | ||
report_format=ReportDataFormat.CSV, | ||
name="report_csv_no_query_context", | ||
) | ||
yield report_schedule | ||
cleanup_report_schedule(report_schedule) | ||
|
||
|
||
@pytest.fixture() | ||
def create_report_email_chart_with_xlsx_no_query_context(): | ||
with app.app_context(): | ||
chart = db.session.query(Slice).first() | ||
chart.query_context = None | ||
report_schedule = create_report_notification( | ||
email_target="[email protected]", | ||
chart=chart, | ||
report_format=ReportDataFormat.XLSX, | ||
name="report_xlsx_no_query_context", | ||
) | ||
yield report_schedule | ||
cleanup_report_schedule(report_schedule) | ||
|
||
|
||
@pytest.fixture() | ||
def create_report_email_dashboard(): | ||
with app.app_context(): | ||
|
@@ -284,7 +313,22 @@ def create_report_slack_chart_with_csv(): | |
report_schedule = create_report_notification( | ||
slack_channel="slack_channel", | ||
chart=chart, | ||
report_format=ReportDataFormat.DATA, | ||
report_format=ReportDataFormat.CSV, | ||
) | ||
yield report_schedule | ||
|
||
cleanup_report_schedule(report_schedule) | ||
|
||
|
||
@pytest.fixture() | ||
def create_report_slack_chart_with_xlsx(): | ||
with app.app_context(): | ||
chart = db.session.query(Slice).first() | ||
chart.query_context = '{"mock": "query_context"}' | ||
report_schedule = create_report_notification( | ||
slack_channel="slack_channel", | ||
chart=chart, | ||
report_format=ReportDataFormat.XLSX, | ||
) | ||
yield report_schedule | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retrieving
df
from csv data it's not the good approach. We need to write a common method for csv and excel where we providedf
param likeget_data(self, df: pd.DataFrame)
insuperset/common/query_context_processor.py
.@dpgaspar Am I correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit confused about this part. We are getting df in order to convert it to excel.
I mean if we already have df we can just convert to excel so what would the common method achieve? Could you provide more details?