You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/07/28 16:15:28 UTC
[GitHub] [superset] AAfghahi opened a new pull request, #20903: feat: test sparkpost
AAfghahi opened a new pull request, #20903:
URL: https://github.com/apache/superset/pull/20903
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
This is testing for a new email feature
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r973296424
##########
superset/reports/notifications/base.py:
##########
@@ -21,11 +21,13 @@
import pandas as pd
from superset.reports.models import ReportRecipients, ReportRecipientType
+from superset.utils.core import HeaderDataType
@dataclass
class NotificationContent:
name: str
+ header_data: HeaderDataType # this is optional to account for error states
Review Comment:
@AAfghahi it looks like this value is always being passed into the class. Can we just remove the comment or is there some other intention here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r942719216
##########
superset/reports/commands/execute.py:
##########
@@ -305,6 +306,27 @@ def _update_query_context(self) -> None:
"Please try loading the chart and saving it again."
) from ex
+ def _get_log_data(self) -> Dict[str, Any]:
+ chart_id = None
+ dashboard_id = None
+ owners = json.loads(self._report_schedule.owners) or None
+ report_source = None
+ if self._report_schedule.chart:
+ report_source = ReportSourceFormat.CHART
+ chart_id = self._report_schedule.chart_id
+ else:
+ report_source = ReportSourceFormat.DASHBOARD
+ dashboard_id = self._report_schedule.dashboard_id
+ log_data = {
+ "notification_type": self._report_schedule.type or None,
Review Comment:
what is a "notification" in this sense?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r942732636
##########
superset/reports/commands/execute.py:
##########
@@ -305,6 +306,27 @@ def _update_query_context(self) -> None:
"Please try loading the chart and saving it again."
) from ex
+ def _get_log_data(self) -> Dict[str, Any]:
+ chart_id = None
+ dashboard_id = None
+ owners = json.loads(self._report_schedule.owners) or None
+ report_source = None
+ if self._report_schedule.chart:
+ report_source = ReportSourceFormat.CHART
+ chart_id = self._report_schedule.chart_id
+ else:
+ report_source = ReportSourceFormat.DASHBOARD
+ dashboard_id = self._report_schedule.dashboard_id
+ log_data = {
+ "notification_type": self._report_schedule.type or None,
Review Comment:
either an alert or a report.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: test sparkpost
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r940746441
##########
superset/reports/commands/execute.py:
##########
@@ -342,23 +343,34 @@ def _get_notification_content(self) -> NotificationContent:
):
embedded_data = self._get_embedded_data()
+ notification_source = None
+ notification_source_id = None
Review Comment:
nit, but can you declare these two at the beginning of the method with the others? It read at first as if you were reassigning the values rather than declaring and I had to check the top of the method to be sure which one it was.
##########
superset/utils/core.py:
##########
@@ -917,6 +918,9 @@ def send_email_smtp( # pylint: disable=invalid-name,too-many-arguments,too-many
msg["Subject"] = subject
msg["From"] = smtp_mail_from
msg["To"] = ", ".join(smtp_mail_to)
+
+ api_object = {"metadata": log_data}
+ msg["X-MSYS-API"] = json.dumps(api_object)
Review Comment:
are you moving these two lines and the `_get_log_data` method to the config?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: test sparkpost
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r940751719
##########
superset/reports/commands/execute.py:
##########
@@ -342,23 +343,34 @@ def _get_notification_content(self) -> NotificationContent:
):
embedded_data = self._get_embedded_data()
+ notification_source = None
+ notification_source_id = None
if self._report_schedule.chart:
name = (
f"{self._report_schedule.name}: "
f"{self._report_schedule.chart.slice_name}"
)
+ notification_source = ReportSourceFormat.CHART
+ notification_source_id = self._report_schedule.chart_id
else:
name = (
f"{self._report_schedule.name}: "
f"{self._report_schedule.dashboard.dashboard_title}"
)
+ notification_source = ReportSourceFormat.DASHBOARDS
+ notification_source_id = self._report_schedule.dashboard_id
+
return NotificationContent(
name=name,
url=url,
screenshots=screenshot_data,
description=self._report_schedule.description,
csv=csv_data,
embedded_data=embedded_data,
+ notification_type=self._report_schedule.type,
+ notification_source=notification_source,
+ notification_source_id=notification_source_id,
+ notification_format=self._report_schedule.report_format,
Review Comment:
I would suggest grouping all of this new content together into either log_data or header_data since it all serves the same function and doesn't need to be passed in separately. It also makes it clear with that naming what it's going to be used for.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: test sparkpost
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r940752277
##########
superset/reports/commands/execute.py:
##########
@@ -342,23 +343,34 @@ def _get_notification_content(self) -> NotificationContent:
):
embedded_data = self._get_embedded_data()
+ notification_source = None
+ notification_source_id = None
if self._report_schedule.chart:
name = (
f"{self._report_schedule.name}: "
f"{self._report_schedule.chart.slice_name}"
)
+ notification_source = ReportSourceFormat.CHART
+ notification_source_id = self._report_schedule.chart_id
else:
name = (
f"{self._report_schedule.name}: "
f"{self._report_schedule.dashboard.dashboard_title}"
)
+ notification_source = ReportSourceFormat.DASHBOARDS
+ notification_source_id = self._report_schedule.dashboard_id
+
return NotificationContent(
name=name,
url=url,
screenshots=screenshot_data,
description=self._report_schedule.description,
csv=csv_data,
embedded_data=embedded_data,
+ notification_type=self._report_schedule.type,
+ notification_source=notification_source,
+ notification_source_id=notification_source_id,
+ notification_format=self._report_schedule.report_format,
Review Comment:
also we can be more flexible about the shape of the data that way, and if it changes, we don't have to update it in many places. The only thing that really relies on the shape is the config method which is optional.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
john-bodley commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r972271380
##########
superset/reports/notifications/base.py:
##########
@@ -21,11 +21,13 @@
import pandas as pd
from superset.reports.models import ReportRecipients, ReportRecipientType
+from superset.utils.core import HeaderDataType
@dataclass
class NotificationContent:
name: str
+ header_data: HeaderDataType # this is optional to account for error states
Review Comment:
@AAfghahi your comment doesn't seem to reflect the actual type and default, i.e., `header_data` is required.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r942719216
##########
superset/reports/commands/execute.py:
##########
@@ -305,6 +306,27 @@ def _update_query_context(self) -> None:
"Please try loading the chart and saving it again."
) from ex
+ def _get_log_data(self) -> Dict[str, Any]:
+ chart_id = None
+ dashboard_id = None
+ owners = json.loads(self._report_schedule.owners) or None
+ report_source = None
+ if self._report_schedule.chart:
+ report_source = ReportSourceFormat.CHART
+ chart_id = self._report_schedule.chart_id
+ else:
+ report_source = ReportSourceFormat.DASHBOARD
+ dashboard_id = self._report_schedule.dashboard_id
+ log_data = {
+ "notification_type": self._report_schedule.type or None,
Review Comment:
what is a "notification" in this sense? I see that term used below, but I wonder if it has the same meaning outside of this context.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r948474707
##########
superset/utils/core.py:
##########
@@ -963,8 +974,10 @@ def send_email_smtp( # pylint: disable=invalid-name,too-many-arguments,too-many
image.add_header("Content-ID", "<%s>" % msgid)
image.add_header("Content-Disposition", "inline")
msg.attach(image)
-
- send_mime_email(smtp_mail_from, recipients, msg, config, dryrun=dryrun)
+ msg_mutator = config["EMAIL_HEADER_MUTATOR"]
+ # the base notification returns the message without any editing.
+ new_msg = msg_mutator(msg, **header_data or {})
Review Comment:
would it be easier to have header_data default to an empty dictionary on line 916? I think this might still fail if `header_data` were None.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r946898402
##########
tests/integration_tests/reports/commands/execute_dashboard_report_tests.py:
##########
@@ -66,3 +67,47 @@ def test_report_for_dashboard_with_tabs(
assert digest == dashboard.digest
assert send_email_smtp_mock.call_count == 1
assert len(send_email_smtp_mock.call_args.kwargs["images"]) == 1
+
+
Review Comment:
Would that exist in this repo or in one with an actual `NOTIFICATION_EMAIL_HEADER_MUTATOR`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r942718191
##########
superset/reports/commands/execute.py:
##########
@@ -305,6 +306,27 @@ def _update_query_context(self) -> None:
"Please try loading the chart and saving it again."
) from ex
+ def _get_log_data(self) -> Dict[str, Any]:
+ chart_id = None
+ dashboard_id = None
+ owners = json.loads(self._report_schedule.owners) or None
Review Comment:
will `self._report_schedule.owners` be a string?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r942714551
##########
superset/config.py:
##########
@@ -1066,6 +1066,12 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument
return sql
+def NOTIFICATION_EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
+ header: Dict[str, Any], **kwargs: Any
Review Comment:
yes, sorry. Will change but will wait for the rest of your review first
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r948475209
##########
superset/utils/core.py:
##########
@@ -184,6 +184,15 @@ class DatasourceType(str, Enum):
VIEW = "view"
+class HeaderDataType(TypedDict):
+ notification_format: str
+ owners: List[int]
+ notification_type: str
+ notification_source: Optional[str]
+ chart_id: Optional[int]
+ dashboard_id: Optional[int]
Review Comment:
all of these are optional, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r942713985
##########
superset/config.py:
##########
@@ -1066,6 +1066,12 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument
return sql
+def NOTIFICATION_EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
+ header: Dict[str, Any], **kwargs: Any
Review Comment:
I believe this is message, right? Rather than header? And is there a class type for the message?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r973296994
##########
tests/integration_tests/reports/commands/execute_dashboard_report_tests.py:
##########
@@ -66,3 +67,47 @@ def test_report_for_dashboard_with_tabs(
assert digest == dashboard.digest
assert send_email_smtp_mock.call_count == 1
assert len(send_email_smtp_mock.call_args.kwargs["images"]) == 1
+
+
Review Comment:
You can mock an example `NOTIFICATION_EMAIL_HEADER_MUTATOR` and test it in this repo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r948478697
##########
superset/utils/core.py:
##########
@@ -963,8 +974,10 @@ def send_email_smtp( # pylint: disable=invalid-name,too-many-arguments,too-many
image.add_header("Content-ID", "<%s>" % msgid)
image.add_header("Content-Disposition", "inline")
msg.attach(image)
-
- send_mime_email(smtp_mail_from, recipients, msg, config, dryrun=dryrun)
+ msg_mutator = config["EMAIL_HEADER_MUTATOR"]
+ # the base notification returns the message without any editing.
+ new_msg = msg_mutator(msg, **header_data or {})
Review Comment:
Yeah! This is something that I learned, you actually are not allowed to make a default into an empty dictionary or list.
##########
superset/reports/commands/execute.py:
##########
@@ -73,6 +74,8 @@
from superset.utils.urls import get_url_path
from superset.utils.webdriver import DashboardStandaloneMode
+from ...utils.core import HeaderDataType
Review Comment:
yeah, I keep on doing this and the black reformats it. So annoying.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r948480448
##########
superset/reports/notifications/base.py:
##########
@@ -21,11 +21,15 @@
import pandas as pd
from superset.reports.models import ReportRecipients, ReportRecipientType
+from superset.utils.core import HeaderDataType
@dataclass
class NotificationContent:
name: str
+ header_data: Optional[
+ HeaderDataType
+ ] = None # this is optional to account for error states
Review Comment:
Yeah, we also create notification_content when there is an error, should we be pushing header data into those cases as well? I assumed we wouldn't be but happy to make it not optional, and create header data in those instances as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #20903: feat: test sparkpost
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #20903:
URL: https://github.com/apache/superset/pull/20903#issuecomment-1198389864
# [Codecov](https://codecov.io/gh/apache/superset/pull/20903?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#20903](https://codecov.io/gh/apache/superset/pull/20903?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9a8d670) into [master](https://codecov.io/gh/apache/superset/commit/2263a76f4d97ef4f3d93e4b45a31e8c3e522e589?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2263a76) will **decrease** coverage by `11.69%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #20903 +/- ##
===========================================
- Coverage 66.32% 54.63% -11.70%
===========================================
Files 1756 1756
Lines 66767 66769 +2
Branches 7060 7060
===========================================
- Hits 44286 36477 -7809
- Misses 20683 28494 +7811
Partials 1798 1798
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `53.11% <0.00%> (-0.01%)` | :arrow_down: |
| python | `57.45% <0.00%> (-24.13%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `50.25% <0.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/20903?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `62.58% <0.00%> (-27.65%)` | :arrow_down: |
| [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-88.89%)` | :arrow_down: |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/key\_value/commands/delete\_expired.py](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZV9leHBpcmVkLnB5) | `0.00% <0.00%> (-80.77%)` | :arrow_down: |
| [superset/dashboards/commands/importers/v0.py](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9pbXBvcnRlcnMvdjAucHk=) | `15.62% <0.00%> (-76.25%)` | :arrow_down: |
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `25.30% <0.00%> (-68.68%)` | :arrow_down: |
| [superset/datasets/commands/create.py](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvY3JlYXRlLnB5) | `29.41% <0.00%> (-68.63%)` | :arrow_down: |
| [superset/datasets/commands/importers/v0.py](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvaW1wb3J0ZXJzL3YwLnB5) | `24.03% <0.00%> (-67.45%)` | :arrow_down: |
| ... and [280 more](https://codecov.io/gh/apache/superset/pull/20903/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] dpgaspar commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r946825971
##########
tests/integration_tests/reports/commands/execute_dashboard_report_tests.py:
##########
@@ -66,3 +67,47 @@ def test_report_for_dashboard_with_tabs(
assert digest == dashboard.digest
assert send_email_smtp_mock.call_count == 1
assert len(send_email_smtp_mock.call_args.kwargs["images"]) == 1
+
+
Review Comment:
consider adding unit tests for `send_email_smtp` with the actual use of `NOTIFICATION_EMAIL_HEADER_MUTATOR`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r946895274
##########
superset/config.py:
##########
@@ -1066,6 +1066,12 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument
return sql
+def NOTIFICATION_EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
+ msg: Dict[str, Any], **kwargs: Any
Review Comment:
One reason we went with `**kwargs` was so that we could keep the information going into the mutator broad and then put the onus of structuring the data on the mutator in `superset_config`. Though thinking about it, this same thought pattern would exist with a `Dict` named parameter. Though in those cases the mutator class would have to account for newer changes, yeah.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r947312557
##########
superset/config.py:
##########
@@ -1066,6 +1066,12 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument
return sql
+def NOTIFICATION_EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
+ msg: Dict[str, Any], **kwargs: Any
+) -> Dict[str, Any]:
+ return msg
Review Comment:
I agree and see the benefits of both. I know that the SQL mutator uses the structure that i am using right now. Should we bring this up to the superset developers at large?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r948491489
##########
superset/reports/commands/execute.py:
##########
@@ -73,6 +74,8 @@
from superset.utils.urls import get_url_path
from superset.utils.webdriver import DashboardStandaloneMode
+from ...utils.core import HeaderDataType
Review Comment:
yeah latest commit fixes it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r948488087
##########
superset/reports/commands/execute.py:
##########
@@ -73,6 +74,8 @@
from superset.utils.urls import get_url_path
from superset.utils.webdriver import DashboardStandaloneMode
+from ...utils.core import HeaderDataType
Review Comment:
+1 please fix this format
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r948467275
##########
superset/reports/commands/execute.py:
##########
@@ -73,6 +74,8 @@
from superset.utils.urls import get_url_path
from superset.utils.webdriver import DashboardStandaloneMode
+from ...utils.core import HeaderDataType
Review Comment:
can you make this absolute instead of relative?
##########
superset/reports/commands/execute.py:
##########
@@ -73,6 +74,8 @@
from superset.utils.urls import get_url_path
from superset.utils.webdriver import DashboardStandaloneMode
+from ...utils.core import HeaderDataType
Review Comment:
can you make this import absolute instead of relative?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r942732410
##########
superset/reports/commands/execute.py:
##########
@@ -305,6 +306,27 @@ def _update_query_context(self) -> None:
"Please try loading the chart and saving it again."
) from ex
+ def _get_log_data(self) -> Dict[str, Any]:
+ chart_id = None
+ dashboard_id = None
+ owners = json.loads(self._report_schedule.owners) or None
Review Comment:
it should be an array
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r942713985
##########
superset/config.py:
##########
@@ -1066,6 +1066,12 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument
return sql
+def NOTIFICATION_EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
+ header: Dict[str, Any], **kwargs: Any
Review Comment:
I believe this is message, right? Rather than header?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: test sparkpost
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r940750628
##########
superset/reports/models.py:
##########
@@ -82,6 +82,11 @@ class ReportCreationMethod(str, enum.Enum):
ALERTS_REPORTS = "alerts_reports"
+class ReportSourceFormat(str, enum.Enum):
+ CHART = "chart"
+ DASHBOARDS = "dashboard"
Review Comment:
nit, plural/singular consistency
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r948479569
##########
superset/utils/core.py:
##########
@@ -184,6 +184,15 @@ class DatasourceType(str, Enum):
VIEW = "view"
+class HeaderDataType(TypedDict):
+ notification_format: str
+ owners: List[int]
+ notification_type: str
+ notification_source: Optional[str]
+ chart_id: Optional[int]
+ dashboard_id: Optional[int]
Review Comment:
Type and format shouldn't be since they are required in order to make a report. I can make all of them optional however.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r948476110
##########
superset/reports/notifications/base.py:
##########
@@ -21,11 +21,15 @@
import pandas as pd
from superset.reports.models import ReportRecipients, ReportRecipientType
+from superset.utils.core import HeaderDataType
@dataclass
class NotificationContent:
name: str
+ header_data: Optional[
+ HeaderDataType
+ ] = None # this is optional to account for error states
Review Comment:
can you explain why this is optional?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] dpgaspar commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r946801284
##########
superset/reports/notifications/base.py:
##########
@@ -32,6 +32,7 @@ class NotificationContent:
description: Optional[str] = ""
url: Optional[str] = None # url to chart/dashboard for this screenshot
embedded_data: Optional[pd.DataFrame] = None
+ header_data: Optional[Dict[Any, Any]] = field(default_factory=lambda: {})
Review Comment:
nit: does `Optional` make sense here?
also can we flesh out the `Dict[Any, Any]` with a `dataclass` or `TypedDict`. If not it should be `Dict[str, Any]`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] dpgaspar commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r946792675
##########
superset/config.py:
##########
@@ -1066,6 +1066,12 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument
return sql
+def NOTIFICATION_EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
+ msg: Dict[str, Any], **kwargs: Any
+) -> Dict[str, Any]:
+ return msg
Review Comment:
Not related, but there seems to be a slight discrepancy on callable config keys for example:
For example `DATASET_HEALTH_CHECK` default is `None` and is declared like this:
```
DATASET_HEALTH_CHECK: Optional[Callable[["SqlaTable"], str]] = None
```
Any thoughts @eschutho @AAfghahi
##########
superset/config.py:
##########
@@ -1066,6 +1066,12 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument
return sql
+def NOTIFICATION_EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
+ msg: Dict[str, Any], **kwargs: Any
Review Comment:
Can you add more info to the docs, with an example use case?
msg is actually a `MIMEMultipart` has the return type also. A bit undecided about `**kwargs` maybe we could make `header_data` a `Dict` named parameter, instead of making it so generic, but still account for `**kwargs` for future changes?
##########
superset/reports/notifications/base.py:
##########
@@ -32,6 +32,7 @@ class NotificationContent:
description: Optional[str] = ""
url: Optional[str] = None # url to chart/dashboard for this screenshot
embedded_data: Optional[pd.DataFrame] = None
+ header_data: Optional[Dict[Any, Any]] = field(default_factory=lambda: {})
Review Comment:
nit: does `Optional` make sense here?
also can we flesh out the `Dict[Any, Any]` with a dataclass or `TypedDict`. If not it should be `Dict[str, Any]`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20903:
URL: https://github.com/apache/superset/pull/20903#discussion_r946907119
##########
superset/reports/notifications/base.py:
##########
@@ -32,6 +32,7 @@ class NotificationContent:
description: Optional[str] = ""
url: Optional[str] = None # url to chart/dashboard for this screenshot
embedded_data: Optional[pd.DataFrame] = None
+ header_data: Optional[Dict[Any, Any]] = field(default_factory=lambda: {})
Review Comment:
Reading through what is in the header_data I think that it doesn't have to be optional you are right. Everything is either conditional or None.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi merged pull request #20903: feat: add header_data into emails
Posted by GitBox <gi...@apache.org>.
AAfghahi merged PR #20903:
URL: https://github.com/apache/superset/pull/20903
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org