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/08/04 21:45:31 UTC

[GitHub] [incubator-superset] JasonD28 opened a new pull request #10519: fix: make SQL-based alert email links user friendly

JasonD28 opened a new pull request #10519:
URL: https://github.com/apache/incubator-superset/pull/10519


   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   The links in SQL-based alert emails were not user friendly, i.e. `local:####/superset`. This PR makes the links user friendly based on `WEBDRIVER_BASEURL_USER_FRIENDLY` in config.py
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TEST PLAN
   <!--- What steps should be taken to verify the changes -->
   - [x] local test
   - [ ] dropbox staging
   - [ ] dropbox production
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Changes UI


----------------------------------------------------------------
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] JasonD28 commented on a change in pull request #10519: fix: make SQL-based alert email links user friendly

Posted by GitBox <gi...@apache.org>.
JasonD28 commented on a change in pull request #10519:
URL: https://github.com/apache/incubator-superset/pull/10519#discussion_r465350346



##########
File path: superset/tasks/schedules.py
##########
@@ -580,13 +580,15 @@ def deliver_alert(alert_id: int, recipients: Optional[str] = None) -> None:
             "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(

Review comment:
       `image_url` was never being used, it was just being replaced on line 591




----------------------------------------------------------------
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] JasonD28 commented on a change in pull request #10519: fix: make SQL-based alert email links user friendly

Posted by GitBox <gi...@apache.org>.
JasonD28 commented on a change in pull request #10519:
URL: https://github.com/apache/incubator-superset/pull/10519#discussion_r465350346



##########
File path: superset/tasks/schedules.py
##########
@@ -580,13 +580,15 @@ def deliver_alert(alert_id: int, recipients: Optional[str] = None) -> None:
             "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(

Review comment:
       `image_url` was never being used, it was just being replaced at https://github.com/apache/incubator-superset/blob/bdfabc23e7b1474c164a55defa4957a027d2b147/superset/tasks/schedules.py#L587-L589




----------------------------------------------------------------
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 merged pull request #10519: fix: make SQL-based alert email links user friendly

Posted by GitBox <gi...@apache.org>.
bkyryliuk merged pull request #10519:
URL: https://github.com/apache/incubator-superset/pull/10519


   


----------------------------------------------------------------
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