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 2020/05/29 07:10:23 UTC
[GitHub] [incubator-superset] mistercrunch opened a new pull request #9944: Alerts! allowing users to set SQL-based email alerts with screenshots
mistercrunch opened a new pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944
This is a collaboration with @robdiciuccio !
<img width="1061" alt="Screen Shot 2020-05-29 at 12 08 26 AM" src="https://user-images.githubusercontent.com/487433/83231655-aff0e380-a140-11ea-8572-1c837fa78e14.png">
<img width="914" alt="Screen Shot 2020-05-29 at 12 07 47 AM" src="https://user-images.githubusercontent.com/487433/83231658-b0897a00-a140-11ea-9031-73dfb1c6a36e.png">
<img width="928" alt="Screen Shot 2020-05-29 at 12 07 41 AM" src="https://user-images.githubusercontent.com/487433/83231659-b1221080-a140-11ea-82f9-6d2f10b62ff9.png">
<img width="1449" alt="Screen Shot 2020-05-29 at 12 07 25 AM" src="https://user-images.githubusercontent.com/487433/83231660-b1221080-a140-11ea-9e6d-79872e8ca61b.png">
----------------------------------------------------------------
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.
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] [incubator-superset] robdiciuccio commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-652045972
Thanks for the ping @bkyryliuk. I'll get this cleaned up and merged.
----------------------------------------------------------------
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.
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] [incubator-superset] robdiciuccio commented on a change in pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on a change in pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#discussion_r448121659
##########
File path: superset/charts/api.py
##########
@@ -479,6 +485,140 @@ def data(self) -> Response:
raise self.response_400(message=f"Unsupported result_format: {result_format}")
+ @expose("/<pk>/cache_screenshot/", methods=["GET"])
+ @protect()
+ @rison(screenshot_query_schema)
+ @safe
+ @statsd_metrics
+ def cache_screenshot(self, pk: int, **kwargs: Dict[str, bool]) -> WerkzeugResponse:
+ """Get Chart screenshot
+ ---
+ get:
+ description: Compute or get already computed screenshot from cache.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ - in: path
+ schema:
+ type: string
+ name: sha
+ responses:
+ 200:
+ description: Chart thumbnail image
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ cache_key:
+ type: string
+ chart_url:
+ type: string
+ image_url:
+ type: string
+ 302:
+ description: Redirects to the current digest
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ rison_dict = kwargs["rison"]
+ window_size = rison_dict.get("window_size") or (800, 600)
+
+ # Don't shrink the image if thumb_size is not specified
+ thumb_size = rison_dict.get("thumb_size") or window_size
+
+ chart = self.datamodel.get(pk, self._base_filters)
+ if not chart:
+ return self.response_404()
+
+ chart_url = get_url_path("Superset.slice", slice_id=chart.id, standalone="true")
+ screenshot_obj = ChartScreenshot(chart_url, chart.digest)
+ cache_key = screenshot_obj.cache_key(window_size, thumb_size)
+ image_url = get_url_path(
+ "ChartRestApi.screenshot", pk=chart.id, digest=cache_key
+ )
+
+ def trigger_celery():
+ logger.info("Triggering screenshot ASYNC")
+ kwargs = {
+ "url": chart_url,
+ "digest": chart.digest,
+ "force": True,
+ "window_size": window_size,
+ "thumb_size": thumb_size,
+ }
+ cache_chart_thumbnail.delay(**kwargs)
+ return self.response(
+ 202, cache_key=cache_key, chart_url=chart_url, image_url=image_url,
+ )
+
+ return trigger_celery()
+
+ @expose("/<pk>/screenshot/<digest>/", methods=["GET"])
+ @protect()
+ @rison(screenshot_query_schema)
+ @safe
+ @statsd_metrics
+ def screenshot(
+ self, pk: int, digest: str = None, **kwargs: Dict[str, bool]
+ ) -> WerkzeugResponse:
+ """Get Chart screenshot
+ ---
+ get:
+ description: Get a computed screenshot from cache.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ - in: path
+ schema:
+ type: string
+ name: digest
+ responses:
+ 200:
+ description: Chart thumbnail image
+ content:
+ image/*:
+ schema:
+ type: string
+ format: binary
+ 302:
+ description: Redirects to the current digest
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ chart = self.datamodel.get(pk, self._base_filters)
+
+ # Making sure the chart still exists
+ if not chart:
+ return self.response_404()
+
+ # TODO make sure the user has access to the chart
Review comment:
We'll need to add the appropriate permissions check here (and to all screenshot endpoints) before this can merge.
----------------------------------------------------------------
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.
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] [incubator-superset] bkyryliuk commented on a change in pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#discussion_r445756450
##########
File path: superset/charts/api.py
##########
@@ -497,12 +637,17 @@ def thumbnail(
if not chart:
return self.response_404()
if kwargs["rison"].get("force", False):
+ logger.info("Triggering thumbnail compute ASYNC")
Review comment:
maybe add chart id to the logging message
##########
File path: superset/models/alerts.py
##########
@@ -0,0 +1,102 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Models for scheduled execution of jobs"""
+from datetime import datetime
+
+from flask_appbuilder import Model
+from sqlalchemy import (
+ Boolean,
+ Column,
+ DateTime,
+ ForeignKey,
+ Integer,
+ String,
+ Table,
+ Text,
+)
+from sqlalchemy.orm import backref, relationship
+
+from superset import security_manager
+
+metadata = Model.metadata # pylint: disable=no-member
+
+
+alert_owner = Table(
+ "alert_owner",
+ metadata,
+ Column("id", Integer, primary_key=True),
+ Column("user_id", Integer, ForeignKey("ab_user.id")),
+ Column("alert_id", Integer, ForeignKey("alerts.id")),
+)
+
+
+class Alert(Model):
Review comment:
would be nice to have AuditMixinNullable
##########
File path: superset/charts/api.py
##########
@@ -453,6 +459,140 @@ def data(self) -> Response:
resp.headers["Content-Type"] = "application/json; charset=utf-8"
return resp
+ @expose("/<pk>/cache_screenshot/", methods=["GET"])
+ @protect()
+ @rison(screenshot_query_schema)
+ @safe
+ @statsd_metrics
+ def cache_screenshot(self, pk: int, **kwargs: Dict[str, bool]) -> WerkzeugResponse:
+ """Get Chart screenshot
+ ---
+ get:
+ description: Compute or get already computed screenshot from cache.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ - in: path
+ schema:
+ type: string
+ name: sha
+ responses:
+ 200:
+ description: Chart thumbnail image
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ cache_key:
+ type: string
+ chart_url:
+ type: string
+ image_url:
+ type: string
+ 302:
+ description: Redirects to the current digest
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ rison_dict = kwargs["rison"]
+ window_size = rison_dict.get("window_size") or (800, 600)
+
+ # Don't shrink the image if thumb_size is not specified
+ thumb_size = rison_dict.get("thumb_size") or window_size
+
+ chart = self.datamodel.get(pk, self._base_filters)
+ if not chart:
+ return self.response_404()
+
+ chart_url = get_url_path("Superset.slice", slice_id=chart.id, standalone="true")
+ screenshot_obj = ChartScreenshot(chart_url, chart.digest)
+ cache_key = screenshot_obj.cache_key(window_size, thumb_size)
+ image_url = get_url_path(
+ "ChartRestApi.screenshot", pk=chart.id, digest=cache_key
+ )
+
+ def trigger_celery():
+ logger.info("Triggering screenshot ASYNC")
+ kwargs = {
+ "url": chart_url,
+ "digest": chart.digest,
+ "force": True,
+ "window_size": window_size,
+ "thumb_size": thumb_size,
+ }
+ cache_chart_thumbnail.delay(**kwargs)
+ return self.response(
+ 202, cache_key=cache_key, chart_url=chart_url, image_url=image_url,
+ )
+
+ return trigger_celery()
+
+ @expose("/<pk>/screenshot/<digest>/", methods=["GET"])
+ @protect()
+ @rison(screenshot_query_schema)
+ @safe
+ @statsd_metrics
+ def screenshot(
Review comment:
s/screenshot/cached_screenshot?
##########
File path: superset/charts/api.py
##########
@@ -453,6 +459,140 @@ def data(self) -> Response:
resp.headers["Content-Type"] = "application/json; charset=utf-8"
return resp
+ @expose("/<pk>/cache_screenshot/", methods=["GET"])
+ @protect()
+ @rison(screenshot_query_schema)
+ @safe
+ @statsd_metrics
+ def cache_screenshot(self, pk: int, **kwargs: Dict[str, bool]) -> WerkzeugResponse:
Review comment:
s/cache_screenshot/schedule_screenshot
##########
File path: superset/tasks/schedules.py
##########
@@ -79,11 +86,13 @@ def _get_recipients(
def _deliver_email(
- schedule: Union[DashboardEmailSchedule, SliceEmailSchedule],
+ schedule: Union[DashboardEmailSchedule, SliceEmailSchedule, Alert],
subject: str,
email: EmailContent,
) -> None:
for (to, bcc) in _get_recipients(schedule):
+ logging.info("Sending email to [%s] bcc [%s]", to, bcc)
Review comment:
maybe do it debug logging? so it is configurable if needed ?
##########
File path: superset/tasks/schedules.py
##########
@@ -475,3 +638,22 @@ def schedule_hourly() -> None:
stop_at = start_at + timedelta(seconds=3600)
schedule_window(ScheduleType.dashboard, start_at, stop_at, resolution)
schedule_window(ScheduleType.slice, start_at, stop_at, resolution)
+
+
+@celery_app.task(name="alerts.schedule_check")
+def schedule_alerts() -> None:
+ """ Celery beat job meant to be invoked every minute to check alerts """
+
+ # if not config["ENABLE_SCHEDULED_EMAIL_REPORTS"]:
Review comment:
probably just delete it, I assume this function should not be called if it's not enabled
##########
File path: superset/charts/api.py
##########
@@ -453,6 +459,140 @@ def data(self) -> Response:
resp.headers["Content-Type"] = "application/json; charset=utf-8"
return resp
+ @expose("/<pk>/cache_screenshot/", methods=["GET"])
+ @protect()
+ @rison(screenshot_query_schema)
+ @safe
+ @statsd_metrics
+ def cache_screenshot(self, pk: int, **kwargs: Dict[str, bool]) -> WerkzeugResponse:
+ """Get Chart screenshot
Review comment:
s/Get Chart screenshot/Schedule screenshot computation in celery
##########
File path: superset/utils/screenshots.py
##########
@@ -218,28 +229,39 @@ def get(
:param thumb_size: Override thumbnail site
"""
payload: Optional[bytes] = None
- thumb_size = thumb_size or self.thumb_size
+ cache_key = self.cache_key(self.window_size, thumb_size)
if cache:
- payload = cache.get(self.cache_key)
+ payload = cache.get(cache_key)
if not payload:
payload = self.compute_and_cache(
user=user, thumb_size=thumb_size, cache=cache
)
else:
- logger.info(f"Loaded thumbnail from cache: {self.cache_key}")
+ logger.info(f"Loaded thumbnail from cache: {cache_key}")
if payload:
return BytesIO(payload)
return None
- def get_from_cache(self, cache: "Cache") -> Optional[BytesIO]:
- payload = cache.get(self.cache_key)
+ def get_from_cache(
+ self, cache: "Cache", window_size=None, thumb_size=None,
+ ) -> Optional[BytesIO]:
+ cache_key = self.cache_key(window_size, thumb_size)
+ payload = cache.get(cache_key)
+ return self.get_from_cache_key(cache, cache_key)
+
+ @staticmethod
+ def get_from_cache_key(cache: "Cache", cache_key: str) -> Optional[BytesIO]:
+ logger.info("Attempting to get from cache: %s", cache_key)
+ payload = cache.get(cache_key)
if payload:
return BytesIO(payload)
+ logger.info("Failed at getting from cache: %s", cache_key)
return None
def compute_and_cache( # pylint: disable=too-many-arguments
self,
user: "User" = None,
+ window_size: Optional[WindowSize] = None,
Review comment:
would be nice to have a timeout here and time to live here
e.g. for hourly alerts ttl should be 1 hour tops
##########
File path: superset/migrations/versions/2f1d15e8a6af_add_alerts.py
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""add_alerts
+
+Revision ID: 2f1d15e8a6af
+Revises: 620241d1153f
+Create Date: 2020-05-26 23:21:50.059635
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "2f1d15e8a6af"
+down_revision = "620241d1153f"
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy.dialects import mysql
+
+
+def upgrade():
+ # ### commands auto generated by Alembic - please adjust! ###
+ op.create_table(
+ "alerts",
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("label", sa.String(length=150), nullable=False),
+ sa.Column("active", sa.Boolean(), nullable=True),
+ sa.Column("crontab", sa.String(length=50), nullable=True),
+ sa.Column("sql", sa.Text(), nullable=True),
+ sa.Column("alert_type", sa.String(length=50), nullable=True),
+ sa.Column("log_retention", sa.Integer(), nullable=False, default=90),
+ sa.Column("grace_period", sa.Integer(), nullable=False, default=60 * 60 * 24),
+ sa.Column("recipients", sa.Text(), nullable=True),
+ sa.Column("slice_id", sa.Integer(), nullable=True),
+ sa.Column("database_id", sa.Integer(), nullable=False),
+ sa.Column("dashboard_id", sa.Integer(), nullable=True),
+ sa.Column("last_eval_dttm", sa.DateTime(), nullable=True),
+ sa.Column("last_state", sa.String(length=10), nullable=True),
+ sa.ForeignKeyConstraint(["dashboard_id"], ["dashboards.id"],),
+ sa.ForeignKeyConstraint(["slice_id"], ["slices.id"],),
+ sa.PrimaryKeyConstraint("id"),
+ )
+ op.create_index(op.f("ix_alerts_active"), "alerts", ["active"], unique=False)
+ op.create_table(
+ "alert_logs",
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("scheduled_dttm", sa.DateTime(), nullable=True),
+ sa.Column("dttm_start", sa.DateTime(), nullable=True),
+ sa.Column("dttm_end", sa.DateTime(), nullable=True),
+ sa.Column("alert_id", sa.Integer(), nullable=True),
+ sa.Column("state", sa.String(length=10), nullable=True),
+ sa.ForeignKeyConstraint(["alert_id"], ["alerts.id"],),
+ sa.PrimaryKeyConstraint("id"),
+ )
+ op.create_table(
+ "alert_owner",
Review comment:
s/alert_owner/alert_owners
if I get it right alert can have many owners
##########
File path: superset/charts/api.py
##########
@@ -453,6 +459,140 @@ def data(self) -> Response:
resp.headers["Content-Type"] = "application/json; charset=utf-8"
return resp
+ @expose("/<pk>/cache_screenshot/", methods=["GET"])
+ @protect()
+ @rison(screenshot_query_schema)
+ @safe
+ @statsd_metrics
+ def cache_screenshot(self, pk: int, **kwargs: Dict[str, bool]) -> WerkzeugResponse:
+ """Get Chart screenshot
+ ---
+ get:
+ description: Compute or get already computed screenshot from cache.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ - in: path
+ schema:
+ type: string
+ name: sha
+ responses:
+ 200:
+ description: Chart thumbnail image
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ cache_key:
+ type: string
+ chart_url:
+ type: string
+ image_url:
+ type: string
+ 302:
+ description: Redirects to the current digest
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ rison_dict = kwargs["rison"]
+ window_size = rison_dict.get("window_size") or (800, 600)
+
+ # Don't shrink the image if thumb_size is not specified
+ thumb_size = rison_dict.get("thumb_size") or window_size
+
+ chart = self.datamodel.get(pk, self._base_filters)
+ if not chart:
+ return self.response_404()
+
+ chart_url = get_url_path("Superset.slice", slice_id=chart.id, standalone="true")
+ screenshot_obj = ChartScreenshot(chart_url, chart.digest)
+ cache_key = screenshot_obj.cache_key(window_size, thumb_size)
+ image_url = get_url_path(
+ "ChartRestApi.screenshot", pk=chart.id, digest=cache_key
+ )
+
+ def trigger_celery():
+ logger.info("Triggering screenshot ASYNC")
+ kwargs = {
+ "url": chart_url,
+ "digest": chart.digest,
+ "force": True,
+ "window_size": window_size,
+ "thumb_size": thumb_size,
+ }
+ cache_chart_thumbnail.delay(**kwargs)
+ return self.response(
+ 202, cache_key=cache_key, chart_url=chart_url, image_url=image_url,
+ )
+
+ return trigger_celery()
+
+ @expose("/<pk>/screenshot/<digest>/", methods=["GET"])
+ @protect()
+ @rison(screenshot_query_schema)
+ @safe
+ @statsd_metrics
+ def screenshot(
+ self, pk: int, digest: str = None, **kwargs: Dict[str, bool]
+ ) -> WerkzeugResponse:
+ """Get Chart screenshot
+ ---
+ get:
+ description: Get a computed screenshot from cache.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ - in: path
+ schema:
+ type: string
+ name: digest
+ responses:
+ 200:
+ description: Chart thumbnail image
+ content:
+ image/*:
+ schema:
+ type: string
+ format: binary
+ 302:
+ description: Redirects to the current digest
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ chart = self.datamodel.get(pk, self._base_filters)
+
+ # Making sure the chart still exists
+ if not chart:
+ return self.response_404()
+
+ # TODO make sure the user has access to the chart
+
+ # fetch the chart screenshot using the current user and cache if set
+ img = ChartScreenshot.get_from_cache_key(thumbnail_cache, digest)
+ if img:
+ return Response(
+ FileWrapper(img), mimetype="image/png", direct_passthrough=True
+ )
+ # TODO: return an empty image
+ return None
Review comment:
maybe return 404 ?
##########
File path: superset/cli.py
##########
@@ -593,3 +593,14 @@ def sync_tags():
add_types(db.engine, metadata)
add_owners(db.engine, metadata)
add_favorites(db.engine, metadata)
+
+
+@with_appcontext
+@superset.command()
Review comment:
delete me :)
it would be useful to have a dev cli for things like it
##########
File path: superset/tasks/schedules.py
##########
@@ -410,6 +419,141 @@ def schedule_email_report( # pylint: disable=unused-argument
raise RuntimeError("Unknown report type")
+@celery_app.task(
+ name="alerts.run_query",
+ bind=True,
+ soft_time_limit=config["EMAIL_ASYNC_TIME_LIMIT_SEC"],
+)
+def schedule_alert_query( # pylint: disable=unused-argument
+ task: Task,
+ report_type: ScheduleType,
+ schedule_id: int,
+ recipients: Optional[str] = None,
+) -> None:
+ model_cls = get_scheduler_model(report_type)
+ dbsession = db.create_scoped_session()
+ schedule = dbsession.query(model_cls).get(schedule_id)
+
+ # The user may have disabled the schedule. If so, ignore this
+ if not schedule or not schedule.active:
+ logger.info("Ignoring deactivated alert")
+ return
+
+ if report_type == ScheduleType.alert:
+ if run_alert_query(schedule, dbsession):
+ # deliver_dashboard OR deliver_slice
+ return
+ else:
+ raise RuntimeError("Unknown report type")
+
+
+class AlertState:
+ ERROR = "error"
+ TRIGGER = "trigger"
+ PASS = "pass"
+
+
+def deliver_alert(alert):
+ logging.info("Triggering alert: %s", alert)
+ img_data = None
+ if alert.slice:
+
+ chart_url = get_url_path(
+ "Superset.slice", slice_id=alert.slice.id, standalone="true"
+ )
+ screenshot = ChartScreenshot(chart_url, alert.slice.digest)
+ cache_key = screenshot.cache_key()
+ image_url = get_url_path(
+ "ChartRestApi.screenshot", pk=alert.slice.id, digest=cache_key
+ )
+
+ user = security_manager.find_user(current_app.config["THUMBNAIL_SELENIUM_USER"])
+ img_data = screenshot.compute_and_cache(
+ user=user, cache=thumbnail_cache, force=True,
+ )
+ else:
+ image_url = "https://media.giphy.com/media/dzaUX7CAG0Ihi/giphy.gif"
+
+ # generate the email
+ subject = f"[Superset] Triggered alert: {alert.label}"
+ data = None
+ images = {"screenshot": img_data}
+ body = __(
+ textwrap.dedent(
+ """\
+ <h2>Alert: %(label)s</h2>
+ <img src="cid:screenshot" alt="%(label)s" />
+ """
+ ),
+ label=alert.label,
+ image_url=image_url,
+ )
+
+ email = EmailContent(body, data, images)
+
+ # send the email
+ _deliver_email(alert, subject, email)
+
+
+def run_alert_query(alert: Alert, session: Session) -> Optional[bool]:
+ """
+ Execute alert.sql and return value if any rows are returned
+ """
+ logger.info(f"Processing alert: {alert}")
+ database = alert.database
+ if not database:
Review comment:
nit: would be nicer to raise 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.
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] [incubator-superset] bkyryliuk commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-651314045
@mistercrunch, @robdiciuccio how would you prefer to take it forward?
This PR touches many files and it would be hard to keep it up to date if this one doesn't get merged.
From my prospective we would love to see it merged and contribute towards alerting in superset, e.g. we would start with adding tests & making it more robust.
----------------------------------------------------------------
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.
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] [incubator-superset] heyuan0403 commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
heyuan0403 commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-658987963
Hi, thanks for adding this great feature ! Does superset have a plan for official release of this ?
----------------------------------------------------------------
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.
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] [incubator-superset] brachipa commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
brachipa commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-702375245
Same for me, it doesn't work, also by trying `superset alert`
I see this exception in logs
```stacktrace
[2020-10-01 20:17:45,040: INFO/ForkPoolWorker-1] Processing alert ID: 2
[2020-10-01 20:17:45,042: DEBUG/ForkPoolWorker-1] Parsing with sqlparse statement: select 1,0,5,55
[2020-10-01 20:17:45,044: INFO/ForkPoolWorker-1] Evaluating SQL for alert <2:brachi>
[2020-10-01 20:17:45,045: DEBUG/ForkPoolWorker-1] Database.get_sqla_engine(). Masked URL: postgresql://superset:XXXXXXXXXX@db:5432/superset
[2020-10-01 20:17:45,054: INFO/ForkPoolWorker-1] Triggering alert: <2:brachi>
[2020-10-01 20:17:45,057: ERROR/ForkPoolWorker-1] Task alerts.run_query[72f7c16b-ba6f-4836-916d-222fb3146641] raised unexpected: OSError(99, 'Cannot assign requested address')
Traceback (most recent call last):
File "/usr/local/lib/python3.6/site-packages/celery/app/trace.py", line 412, in trace_task
R = retval = fun(*args, **kwargs)
File "/app/superset/app.py", line 115, in __call__
return task_base.__call__(self, *args, **kwargs)
File "/usr/local/lib/python3.6/site-packages/celery/app/trace.py", line 704, in __protected_call__
return self.run(*args, **kwargs)
File "/app/superset/tasks/schedules.py", line 554, in schedule_alert_query
if run_alert_query(schedule, dbsession):
File "/app/superset/tasks/schedules.py", line 648, in run_alert_query
deliver_alert(alert)
File "/app/superset/tasks/schedules.py", line 607, in deliver_alert
_deliver_email(alert.recipients, deliver_as_group, subject, body, data, images)
File "/app/superset/tasks/schedules.py", line 132, in _deliver_email
dryrun=config["SCHEDULED_EMAIL_DEBUG_MODE"],
File "/app/superset/utils/core.py", line 787, in send_email_smtp
send_mime_email(smtp_mail_from, recipients, msg, config, dryrun=dryrun)
File "/app/superset/utils/core.py", line 808, in send_mime_email
else smtplib.SMTP(smtp_host, smtp_port)
File "/usr/local/lib/python3.6/smtplib.py", line 251, in __init__
(code, msg) = self.connect(host, port)
File "/usr/local/lib/python3.6/smtplib.py", line 336, in connect
self.sock = self._get_socket(host, port, self.timeout)
File "/usr/local/lib/python3.6/smtplib.py", line 307, in _get_socket
self.source_address)
File "/usr/local/lib/python3.6/socket.py", line 724, in create_connection
raise err
File "/usr/local/lib/python3.6/socket.py", line 713, in create_connection
sock.connect(sa)
OSError: [Errno 99] Cannot assign requested address
```
----------------------------------------------------------------
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.
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] [incubator-superset] rumbin commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
rumbin commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-638617885
Great feature! I'm looking forward to having it in an official release :-)
----------------------------------------------------------------
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.
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] [incubator-superset] robdiciuccio commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-654440392
@bkyryliuk @willbarrett this looks good to me as a first pass, with the understanding that @bkyryliuk's team will be iterating on it. If it LGTY, feel free to merge.
----------------------------------------------------------------
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.
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] [incubator-superset] mistercrunch commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-665449640
There's a CLI command `superset alert` that runs a scheduler loop and can help test the feature.
----------------------------------------------------------------
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.
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] [incubator-superset] kkalyan commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
kkalyan commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-649911569
Nice! SQL check is powerful and compact.
Does this replace current Email notification feature for dashboards and slices?
----------------------------------------------------------------
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.
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] [incubator-superset] bkyryliuk commented on a change in pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#discussion_r445763311
##########
File path: superset/utils/screenshots.py
##########
@@ -218,28 +229,39 @@ def get(
:param thumb_size: Override thumbnail site
"""
payload: Optional[bytes] = None
- thumb_size = thumb_size or self.thumb_size
+ cache_key = self.cache_key(self.window_size, thumb_size)
if cache:
- payload = cache.get(self.cache_key)
+ payload = cache.get(cache_key)
if not payload:
payload = self.compute_and_cache(
user=user, thumb_size=thumb_size, cache=cache
)
else:
- logger.info(f"Loaded thumbnail from cache: {self.cache_key}")
+ logger.info(f"Loaded thumbnail from cache: {cache_key}")
if payload:
return BytesIO(payload)
return None
- def get_from_cache(self, cache: "Cache") -> Optional[BytesIO]:
- payload = cache.get(self.cache_key)
+ def get_from_cache(
+ self, cache: "Cache", window_size=None, thumb_size=None,
+ ) -> Optional[BytesIO]:
+ cache_key = self.cache_key(window_size, thumb_size)
+ payload = cache.get(cache_key)
+ return self.get_from_cache_key(cache, cache_key)
+
+ @staticmethod
+ def get_from_cache_key(cache: "Cache", cache_key: str) -> Optional[BytesIO]:
+ logger.info("Attempting to get from cache: %s", cache_key)
+ payload = cache.get(cache_key)
if payload:
return BytesIO(payload)
+ logger.info("Failed at getting from cache: %s", cache_key)
return None
def compute_and_cache( # pylint: disable=too-many-arguments
self,
user: "User" = None,
+ window_size: Optional[WindowSize] = None,
Review comment:
would be nice to have a timeout and time to live here
e.g. for hourly alerts ttl should be 1 hour tops
----------------------------------------------------------------
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.
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] [incubator-superset] bkyryliuk commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-654484642
yep, big thanks @robdiciuccio and @willbarrett !
----------------------------------------------------------------
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.
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] [incubator-superset] mistercrunch commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-654564365
----------------------------------------------------------------
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.
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] [incubator-superset] robdiciuccio commented on a change in pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on a change in pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#discussion_r449965034
##########
File path: superset/tasks/schedules.py
##########
@@ -410,6 +419,141 @@ def schedule_email_report( # pylint: disable=unused-argument
raise RuntimeError("Unknown report type")
+@celery_app.task(
+ name="alerts.run_query",
+ bind=True,
+ soft_time_limit=config["EMAIL_ASYNC_TIME_LIMIT_SEC"],
+)
+def schedule_alert_query( # pylint: disable=unused-argument
+ task: Task,
+ report_type: ScheduleType,
+ schedule_id: int,
+ recipients: Optional[str] = None,
+) -> None:
+ model_cls = get_scheduler_model(report_type)
+ dbsession = db.create_scoped_session()
+ schedule = dbsession.query(model_cls).get(schedule_id)
+
+ # The user may have disabled the schedule. If so, ignore this
+ if not schedule or not schedule.active:
+ logger.info("Ignoring deactivated alert")
+ return
+
+ if report_type == ScheduleType.alert:
+ if run_alert_query(schedule, dbsession):
+ # deliver_dashboard OR deliver_slice
+ return
+ else:
+ raise RuntimeError("Unknown report type")
+
+
+class AlertState:
+ ERROR = "error"
+ TRIGGER = "trigger"
+ PASS = "pass"
+
+
+def deliver_alert(alert):
+ logging.info("Triggering alert: %s", alert)
+ img_data = None
+ if alert.slice:
+
+ chart_url = get_url_path(
+ "Superset.slice", slice_id=alert.slice.id, standalone="true"
+ )
+ screenshot = ChartScreenshot(chart_url, alert.slice.digest)
+ cache_key = screenshot.cache_key()
+ image_url = get_url_path(
+ "ChartRestApi.screenshot", pk=alert.slice.id, digest=cache_key
+ )
+
+ user = security_manager.find_user(current_app.config["THUMBNAIL_SELENIUM_USER"])
+ img_data = screenshot.compute_and_cache(
+ user=user, cache=thumbnail_cache, force=True,
+ )
+ else:
+ image_url = "https://media.giphy.com/media/dzaUX7CAG0Ihi/giphy.gif"
+
+ # generate the email
+ subject = f"[Superset] Triggered alert: {alert.label}"
+ data = None
+ images = {"screenshot": img_data}
+ body = __(
+ textwrap.dedent(
+ """\
+ <h2>Alert: %(label)s</h2>
+ <img src="cid:screenshot" alt="%(label)s" />
+ """
+ ),
+ label=alert.label,
+ image_url=image_url,
+ )
+
+ email = EmailContent(body, data, images)
+
+ # send the email
+ _deliver_email(alert, subject, email)
+
+
+def run_alert_query(alert: Alert, session: Session) -> Optional[bool]:
+ """
+ Execute alert.sql and return value if any rows are returned
+ """
+ logger.info(f"Processing alert: {alert}")
+ database = alert.database
+ if not database:
Review comment:
We likely don't want celery to retry the failed jobs in this case.
----------------------------------------------------------------
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.
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] [incubator-superset] willbarrett merged pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
willbarrett merged pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-commenter commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-654402247
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9944?src=pr&el=h1) Report
> Merging [#9944](https://codecov.io/gh/apache/incubator-superset/pull/9944?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/96647054351535843dc802401a834f64358e99e4&el=desc) will **increase** coverage by `3.85%`.
> The diff coverage is `46.89%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9944?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9944 +/- ##
==========================================
+ Coverage 65.69% 69.55% +3.85%
==========================================
Files 594 194 -400
Lines 31497 18765 -12732
Branches 3223 0 -3223
==========================================
- Hits 20692 13052 -7640
+ Misses 10625 5713 -4912
+ Partials 180 0 -180
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `?` | |
| #python | `69.55% <46.89%> (-0.61%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9944?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvX19pbml0X18ucHk=) | `100.00% <ø> (ø)` | |
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/dashboards/api.py](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9hcGkucHk=) | `80.76% <20.00%> (-0.40%)` | :arrow_down: |
| [superset/tasks/schedules.py](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdGFza3Mvc2NoZWR1bGVzLnB5) | `61.56% <21.78%> (-18.35%)` | :arrow_down: |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `72.24% <30.23%> (-8.08%)` | :arrow_down: |
| [superset/utils/screenshots.py](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvc2NyZWVuc2hvdHMucHk=) | `29.41% <32.50%> (-2.93%)` | :arrow_down: |
| [superset/models/slice.py](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NsaWNlLnB5) | `84.65% <33.33%> (-0.38%)` | :arrow_down: |
| [superset/tasks/thumbnails.py](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdGFza3MvdGh1bWJuYWlscy5weQ==) | `43.33% <33.33%> (+2.59%)` | :arrow_up: |
| [superset/cli.py](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2xpLnB5) | `39.60% <38.46%> (-0.13%)` | :arrow_down: |
| [superset/app.py](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXBwLnB5) | `81.02% <50.00%> (-0.80%)` | :arrow_down: |
| ... and [440 more](https://codecov.io/gh/apache/incubator-superset/pull/9944/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9944?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9944?src=pr&el=footer). Last update [9664705...f7c7e36](https://codecov.io/gh/apache/incubator-superset/pull/9944?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] willbarrett commented on a change in pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#discussion_r445162640
##########
File path: superset/app.py
##########
@@ -379,6 +383,17 @@ def init_views(self) -> None:
icon="fa-search",
)
+ if self.config["ENABLE_ALERTS"]:
Review comment:
Should this be a config flag or a feature flag?
##########
File path: superset/tasks/schedules.py
##########
@@ -79,11 +86,13 @@ def _get_recipients(
def _deliver_email(
- schedule: Union[DashboardEmailSchedule, SliceEmailSchedule],
+ schedule: Union[DashboardEmailSchedule, SliceEmailSchedule, Alert],
subject: str,
email: EmailContent,
) -> None:
for (to, bcc) in _get_recipients(schedule):
+ logging.info("Sending email to [%s] bcc [%s]", to, bcc)
Review comment:
Let's not write email addresses to logs. This is protected information in most organizations.
##########
File path: superset/charts/schemas.py
##########
@@ -27,9 +27,22 @@
# RISON/JSON schemas for query parameters
#
get_delete_ids_schema = {"type": "array", "items": {"type": "integer"}}
+width_heiht_schema = {
Review comment:
nit: typo - should be `width_height_schema`
----------------------------------------------------------------
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.
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] [incubator-superset] mistercrunch commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-702424715
Email configuration doesn't seem to work here.Looks like you need to open your SMTP port
----------------------------------------------------------------
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.
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] [incubator-superset] robdiciuccio commented on a change in pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on a change in pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#discussion_r450356602
##########
File path: superset/charts/api.py
##########
@@ -479,6 +485,140 @@ def data(self) -> Response:
raise self.response_400(message=f"Unsupported result_format: {result_format}")
+ @expose("/<pk>/cache_screenshot/", methods=["GET"])
+ @protect()
+ @rison(screenshot_query_schema)
+ @safe
+ @statsd_metrics
+ def cache_screenshot(self, pk: int, **kwargs: Dict[str, bool]) -> WerkzeugResponse:
+ """Get Chart screenshot
+ ---
+ get:
+ description: Compute or get already computed screenshot from cache.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ - in: path
+ schema:
+ type: string
+ name: sha
+ responses:
+ 200:
+ description: Chart thumbnail image
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ cache_key:
+ type: string
+ chart_url:
+ type: string
+ image_url:
+ type: string
+ 302:
+ description: Redirects to the current digest
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ rison_dict = kwargs["rison"]
+ window_size = rison_dict.get("window_size") or (800, 600)
+
+ # Don't shrink the image if thumb_size is not specified
+ thumb_size = rison_dict.get("thumb_size") or window_size
+
+ chart = self.datamodel.get(pk, self._base_filters)
+ if not chart:
+ return self.response_404()
+
+ chart_url = get_url_path("Superset.slice", slice_id=chart.id, standalone="true")
+ screenshot_obj = ChartScreenshot(chart_url, chart.digest)
+ cache_key = screenshot_obj.cache_key(window_size, thumb_size)
+ image_url = get_url_path(
+ "ChartRestApi.screenshot", pk=chart.id, digest=cache_key
+ )
+
+ def trigger_celery():
+ logger.info("Triggering screenshot ASYNC")
+ kwargs = {
+ "url": chart_url,
+ "digest": chart.digest,
+ "force": True,
+ "window_size": window_size,
+ "thumb_size": thumb_size,
+ }
+ cache_chart_thumbnail.delay(**kwargs)
+ return self.response(
+ 202, cache_key=cache_key, chart_url=chart_url, image_url=image_url,
+ )
+
+ return trigger_celery()
+
+ @expose("/<pk>/screenshot/<digest>/", methods=["GET"])
+ @protect()
+ @rison(screenshot_query_schema)
+ @safe
+ @statsd_metrics
+ def screenshot(
+ self, pk: int, digest: str = None, **kwargs: Dict[str, bool]
+ ) -> WerkzeugResponse:
+ """Get Chart screenshot
+ ---
+ get:
+ description: Get a computed screenshot from cache.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ - in: path
+ schema:
+ type: string
+ name: digest
+ responses:
+ 200:
+ description: Chart thumbnail image
+ content:
+ image/*:
+ schema:
+ type: string
+ format: binary
+ 302:
+ description: Redirects to the current digest
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ chart = self.datamodel.get(pk, self._base_filters)
+
+ # Making sure the chart still exists
+ if not chart:
+ return self.response_404()
+
+ # TODO make sure the user has access to the chart
Review comment:
Because these endpoints are behind the `THUMBNAILS` feature flag, which defaults to `False`, should be OK to merge now in order to unblock.
----------------------------------------------------------------
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.
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] [incubator-superset] camilo-uptime commented on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
camilo-uptime commented on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-665377349
Hi!, thanks for this feature!, I've been trying to test it with no success I enabled the feature on the config, and created a dummy alert, spawned a celery worker and beat but nothing happens, did I miss something?, I have chart and dashboard reports working properly :(.
----------------------------------------------------------------
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.
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] [incubator-superset] brachipa edited a comment on pull request #9944: feat: Alerts! allowing users to set SQL-based email alerts with screenshots
Posted by GitBox <gi...@apache.org>.
brachipa edited a comment on pull request #9944:
URL: https://github.com/apache/incubator-superset/pull/9944#issuecomment-702375245
Same for me, it doesn't work, also by trying `superset alert` (release: 0.37.2)
I see this exception in logs
```stacktrace
[2020-10-01 20:17:45,040: INFO/ForkPoolWorker-1] Processing alert ID: 2
[2020-10-01 20:17:45,042: DEBUG/ForkPoolWorker-1] Parsing with sqlparse statement: select 1,0,5,55
[2020-10-01 20:17:45,044: INFO/ForkPoolWorker-1] Evaluating SQL for alert <2:brachi>
[2020-10-01 20:17:45,045: DEBUG/ForkPoolWorker-1] Database.get_sqla_engine(). Masked URL: postgresql://superset:XXXXXXXXXX@db:5432/superset
[2020-10-01 20:17:45,054: INFO/ForkPoolWorker-1] Triggering alert: <2:brachi>
[2020-10-01 20:17:45,057: ERROR/ForkPoolWorker-1] Task alerts.run_query[72f7c16b-ba6f-4836-916d-222fb3146641] raised unexpected: OSError(99, 'Cannot assign requested address')
Traceback (most recent call last):
File "/usr/local/lib/python3.6/site-packages/celery/app/trace.py", line 412, in trace_task
R = retval = fun(*args, **kwargs)
File "/app/superset/app.py", line 115, in __call__
return task_base.__call__(self, *args, **kwargs)
File "/usr/local/lib/python3.6/site-packages/celery/app/trace.py", line 704, in __protected_call__
return self.run(*args, **kwargs)
File "/app/superset/tasks/schedules.py", line 554, in schedule_alert_query
if run_alert_query(schedule, dbsession):
File "/app/superset/tasks/schedules.py", line 648, in run_alert_query
deliver_alert(alert)
File "/app/superset/tasks/schedules.py", line 607, in deliver_alert
_deliver_email(alert.recipients, deliver_as_group, subject, body, data, images)
File "/app/superset/tasks/schedules.py", line 132, in _deliver_email
dryrun=config["SCHEDULED_EMAIL_DEBUG_MODE"],
File "/app/superset/utils/core.py", line 787, in send_email_smtp
send_mime_email(smtp_mail_from, recipients, msg, config, dryrun=dryrun)
File "/app/superset/utils/core.py", line 808, in send_mime_email
else smtplib.SMTP(smtp_host, smtp_port)
File "/usr/local/lib/python3.6/smtplib.py", line 251, in __init__
(code, msg) = self.connect(host, port)
File "/usr/local/lib/python3.6/smtplib.py", line 336, in connect
self.sock = self._get_socket(host, port, self.timeout)
File "/usr/local/lib/python3.6/smtplib.py", line 307, in _get_socket
self.source_address)
File "/usr/local/lib/python3.6/socket.py", line 724, in create_connection
raise err
File "/usr/local/lib/python3.6/socket.py", line 713, in create_connection
sock.connect(sa)
OSError: [Errno 99] Cannot assign requested address
```
----------------------------------------------------------------
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.
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