Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed Scheduled Queries Report #3798

Merged
merged 67 commits into from
Jul 28, 2019
Merged

Failed Scheduled Queries Report #3798

merged 67 commits into from
Jul 28, 2019

Conversation

rauchy
Copy link
Contributor

@rauchy rauchy commented May 14, 2019

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • New Query Runner (Data Source)
  • New Alert Destination
  • Other

Description

This PR sends out an aggregated e-mail report for failing scheduled queries as suggested in #3797.

Related Tickets & Documents

#3797

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

image

@rauchy rauchy marked this pull request as ready for review May 19, 2019 09:12
@rauchy rauchy requested a review from arikfr May 19, 2019 09:12
@ranbena
Copy link
Contributor

ranbena commented May 19, 2019

@rauchy if you wish to collaborate on the layout and design of the report, plz open a discuss topic.

@rauchy
Copy link
Contributor Author

rauchy commented May 21, 2019

Updated code and PR screenshot to @ranbena's design ❤️

@rauchy
Copy link
Contributor Author

rauchy commented Jul 14, 2019

@arikfr any plans to merge this?

redash/settings/__init__.py Show resolved Hide resolved
redash/tasks/failure_report.py Outdated Show resolved Hide resolved
redash/tasks/failure_report.py Outdated Show resolved Hide resolved
redash/tasks/failure_report.py Outdated Show resolved Hide resolved
redash/tasks/failure_report.py Outdated Show resolved Hide resolved
redash/tasks/failure_report.py Outdated Show resolved Hide resolved
redash/tasks/failure_report.py Outdated Show resolved Hide resolved
redash/tasks/failure_report.py Outdated Show resolved Hide resolved
@rauchy rauchy requested a review from arikfr July 21, 2019 05:52
@rauchy rauchy merged commit 7fb33e3 into master Jul 28, 2019
@rauchy rauchy deleted the failed-queries-report branch July 28, 2019 09:45
harveyrendell pushed a commit to pushpay/redash that referenced this pull request Nov 14, 2019
* initial work on e-mail report for failed queries

* send failure report only for scheduled queries and not for adhoc queries

* add setting to determine if to send failure reports

* add setting to determine interval of aggregated e-mail report

* html templating of scheduled query failure report

* break line

* support timeouts for failure reports

* aggregate errors within message and warn if approaching threshold

* handle errors in QueryExecutor.run instead of on_failure

* move failure report to its own module

* indicate that failure count is since last report

* copy changes

* format with <code>

* styling, copy and add a link to the query instead of the query text

* separate reports with <hr>

* switch to UTC

* move <h2> to actual e-mail subject

* add explicit message for SoftTimeLimitExceeded

* fix test to use soft time limits

* default query failure threshold to 100

* use base_url from utils

* newlines. newlines everywhere.

* remove redundant import

* apply new design for failure report

* use jinja to format the failure report

* don't show comment block if no comment is provided

* don't send emails if, for some reason, there are no available errors

* subtract 1 from failure count, because the first one is represented by 'Last failed'

* don't show '+X more failures' if there's only one

* extract subject to variable

* format as text, while we're at it

* allow scrolling for long exception messages

* test that e-mails are scheduled only  when beneath limit

* test for indicating when approaching report limits + refactor

* test that failures are aggregated

* test that report counts per query and reason

* test that the latest failure occurence is reported

* force sending reports for testing purposes

* Update redash/templates/emails/failures.html

Co-Authored-By: Ran Byron <ranbena@gmail.com>

* Update redash/templates/emails/failures.html

Co-Authored-By: Ran Byron <ranbena@gmail.com>

* Update redash/tasks/failure_report.py

* add org setting for email reports

* remove logo from failure report email

* correctly use the organization setting for sending failure reports

* use user id as key for failure reports data structure

* Update redash/tasks/failure_report.py

Co-Authored-By: Arik Fraimovich <arik@arikfr.com>

* build comments while creating context for e-mail templates

* figure out the base url when creating the e-mail

* no need to expire pending failure report keys as they are deleted anyway when sent

* a couple of CodeClimate changes

* refactor key creationg to a single location

* refactor tests to send e-mail from a single function

* use beat to schedule a periodic send_aggregated_errors task instead of using countdown per email

* remove pending key as it is no longer required when a periodic task picks up the reports to send

* a really important blank line. REALLY important.

* Revert "a really important blank line. REALLY important."

This reverts commit c7d8ed8.

* a really important blank line. REALLY important. It is the best blank line.

* don't send failure emails to disabled users
susodapop added a commit to getredash/website that referenced this pull request Apr 24, 2020
arikfr pushed a commit to getredash/website that referenced this pull request Apr 28, 2020
* Updates doc to reflect getredash/redash#4182

* Revises grammar.

* Implements Arik's feedback from #245

* Optimised images with calibre/image-actions

* Adds note about scheduled query failure reports.

Closes #372

* Adds screenshot of failure report email.

Taken from getredash/redash#3798

* Optimised images with calibre/image-actions

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants