From dae601318bb316129323bed5f6b4eb90b1a83f6c Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 14 May 2019 09:26:53 +0300 Subject: [PATCH 01/58] initial work on e-mail report for failed queries --- redash/tasks/queries.py | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index 6002ccd275..95137760dd 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -1,6 +1,7 @@ import logging import signal import time +import datetime import redis from celery.exceptions import SoftTimeLimitExceeded, TimeLimitExceeded @@ -11,7 +12,8 @@ from redash import models, redis_connection, settings, statsd_client from redash.query_runner import InterruptException from redash.tasks.alerts import check_alerts_for_query -from redash.utils import gen_query_hash, json_dumps, utcnow, mustache_render +from redash.tasks.general import send_mail +from redash.utils import gen_query_hash, json_dumps, json_loads, utcnow, mustache_render from redash.worker import celery logger = get_task_logger(__name__) @@ -385,10 +387,41 @@ def _load_data_source(self): logger.info("task=execute_query state=load_ds ds_id=%d", self.data_source_id) return models.DataSource.query.get(self.data_source_id) +@celery.task(name="redash.tasks.send_aggregated_errors") +def send_aggregated_errors(email_address): + key = 'aggregated_failures:{}'.format(email_address) + errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] + + text = "We're sorry, but these queries failed over the past hour:\n" + \ + '\n'.join(['\nQuery: {}\nFailed at: {}\nFailure reason: {}'.format(e['query'], e['failed_at'], e['message']) for e in errors]) + send_mail.delay([email_address], "Failed Queries", None, text) + + redis_connection.delete(key) + +def notify_of_failure(self, exc, _, args, __, ___): + if not settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES: + return + + query, _, metadata = args[0:3] + email_address = metadata.get('Username') + + if email_address: + key = 'aggregated_failures:{}'.format(email_address) + redis_connection.lpush(key, json_dumps({ + 'query': query, + 'message': exc.message, + 'failed_at': datetime.datetime.now().strftime("%Y-%m-%d %H:%M") + })) + + if not redis_connection.exists('{}:pending'.format(key)): + delay = 10 + send_aggregated_errors.apply_async(args=(email_address,), countdown=delay) + redis_connection.set('{}:pending'.format(key), 1) + redis_connection.expire('{}:pending'.format(key), delay) # user_id is added last as a keyword argument for backward compatability -- to support executing previously submitted # jobs before the upgrade to this version. -@celery.task(name="redash.tasks.execute_query", bind=True, track_started=True) +@celery.task(name="redash.tasks.execute_query", bind=True, track_started=True, on_failure=notify_of_failure) def execute_query(self, query, data_source_id, metadata, user_id=None, scheduled_query_id=None, is_api_key=False): if scheduled_query_id is not None: From ca7e3a28cbfe309b099281c7395b78251e5f8659 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 14 May 2019 09:58:42 +0300 Subject: [PATCH 02/58] send failure report only for scheduled queries and not for adhoc queries --- redash/tasks/queries.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index 95137760dd..0049bc7de3 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -399,16 +399,17 @@ def send_aggregated_errors(email_address): redis_connection.delete(key) def notify_of_failure(self, exc, _, args, __, ___): - if not settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES: + scheduled_query_id = args[-2] + if scheduled_query_id is None or not settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES: return - query, _, metadata = args[0:3] - email_address = metadata.get('Username') + query = models.Query.query.get(scheduled_query_id) + email_address = query.user.email if email_address: key = 'aggregated_failures:{}'.format(email_address) redis_connection.lpush(key, json_dumps({ - 'query': query, + 'query': query.query_text, 'message': exc.message, 'failed_at': datetime.datetime.now().strftime("%Y-%m-%d %H:%M") })) From b388c6f6a22741ab889610587bd5f1bfa9403134 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 14 May 2019 10:00:37 +0300 Subject: [PATCH 03/58] add setting to determine if to send failure reports --- redash/settings/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/redash/settings/__init__.py b/redash/settings/__init__.py index 864d04385a..ddfd938397 100644 --- a/redash/settings/__init__.py +++ b/redash/settings/__init__.py @@ -212,6 +212,8 @@ def email_server_is_configured(): HOST = os.environ.get('REDASH_HOST', '') +SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES = parse_boolean(os.environ.get('REDASH_SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES', 'false')) + ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE = os.environ.get('REDASH_ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE', "({state}) {alert_name}") # How many requests are allowed per IP to the login page before From e910fd3e6d782c5cb38b489063eb9ae4c1bf742f Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 14 May 2019 10:06:44 +0300 Subject: [PATCH 04/58] add setting to determine interval of aggregated e-mail report --- redash/settings/__init__.py | 1 + redash/tasks/queries.py | 7 +++---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/redash/settings/__init__.py b/redash/settings/__init__.py index ddfd938397..b155956880 100644 --- a/redash/settings/__init__.py +++ b/redash/settings/__init__.py @@ -213,6 +213,7 @@ def email_server_is_configured(): HOST = os.environ.get('REDASH_HOST', '') SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES = parse_boolean(os.environ.get('REDASH_SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES', 'false')) +SEND_FAILURE_EMAIL_INTERVAL = int(os.environ.get('REDASH_SEND_FAILURE_EMAIL_INTERVAL', 3600)) ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE = os.environ.get('REDASH_ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE', "({state}) {alert_name}") diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index 0049bc7de3..f7fc08f25c 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -392,7 +392,7 @@ def send_aggregated_errors(email_address): key = 'aggregated_failures:{}'.format(email_address) errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] - text = "We're sorry, but these queries failed over the past hour:\n" + \ + text = "We're sorry, but these queries failed lately:\n" + \ '\n'.join(['\nQuery: {}\nFailed at: {}\nFailure reason: {}'.format(e['query'], e['failed_at'], e['message']) for e in errors]) send_mail.delay([email_address], "Failed Queries", None, text) @@ -415,10 +415,9 @@ def notify_of_failure(self, exc, _, args, __, ___): })) if not redis_connection.exists('{}:pending'.format(key)): - delay = 10 - send_aggregated_errors.apply_async(args=(email_address,), countdown=delay) + send_aggregated_errors.apply_async(args=(email_address,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) redis_connection.set('{}:pending'.format(key), 1) - redis_connection.expire('{}:pending'.format(key), delay) + redis_connection.expire('{}:pending'.format(key), settings.SEND_FAILURE_EMAIL_INTERVAL) # user_id is added last as a keyword argument for backward compatability -- to support executing previously submitted # jobs before the upgrade to this version. From e6955497a48260f66056906c3040268534370e39 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 14 May 2019 11:26:52 +0300 Subject: [PATCH 05/58] html templating of scheduled query failure report --- redash/tasks/queries.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index f7fc08f25c..60ef60374d 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -392,11 +392,10 @@ def send_aggregated_errors(email_address): key = 'aggregated_failures:{}'.format(email_address) errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] - text = "We're sorry, but these queries failed lately:\n" + \ - '\n'.join(['\nQuery: {}\nFailed at: {}\nFailure reason: {}'.format(e['query'], e['failed_at'], e['message']) for e in errors]) - send_mail.delay([email_address], "Failed Queries", None, text) - - redis_connection.delete(key) + html = "We're sorry, but these queries failed lately:
  1. {}
".format( + '
  • '.join(['Failed at: {}
    Failure reason: {}
    Query: {}'.format(e['failed_at'], e['message'], e['query']) for e in errors]) + ) + send_mail.delay([email_address], "Uh-oh, Some Scheduled Queries Failed!", html, None) redis_connection.delete(key) def notify_of_failure(self, exc, _, args, __, ___): scheduled_query_id = args[-2] From 13e9bb11ec2dc1c9f18e675e5430d627f8c5ea7c Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 14 May 2019 11:35:53 +0300 Subject: [PATCH 06/58] break line --- redash/tasks/queries.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index 60ef60374d..7712628df0 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -395,7 +395,9 @@ def send_aggregated_errors(email_address): html = "We're sorry, but these queries failed lately:
    1. {}
    ".format( '
  • '.join(['Failed at: {}
    Failure reason: {}
    Query: {}'.format(e['failed_at'], e['message'], e['query']) for e in errors]) ) - send_mail.delay([email_address], "Uh-oh, Some Scheduled Queries Failed!", html, None) redis_connection.delete(key) + send_mail.delay([email_address], "Uh-oh, Some Scheduled Queries Failed!", html, None) + + redis_connection.delete(key) def notify_of_failure(self, exc, _, args, __, ___): scheduled_query_id = args[-2] From 95be1a98910e0ff56db5e477d0551a71c0ad8038 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 14 May 2019 12:34:58 +0300 Subject: [PATCH 07/58] support timeouts for failure reports --- redash/tasks/queries.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index 7712628df0..a94ca63fd4 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -17,7 +17,7 @@ from redash.worker import celery logger = get_task_logger(__name__) - +TIMEOUT_MESSAGE = "Query exceeded Redash query execution time limit." def _job_lock_id(query_hash, data_source_id): return "query_hash_job:%s:%s" % (data_source_id, query_hash) @@ -58,7 +58,7 @@ def to_dict(self): status = self.STATUSES[task_status] if isinstance(result, (TimeLimitExceeded, SoftTimeLimitExceeded)): - error = "Query exceeded Redash query execution time limit." + error = TIMEOUT_MESSAGE status = 4 elif isinstance(result, Exception): error = result.message @@ -144,7 +144,7 @@ def enqueue_query(query, data_source, user_id, is_api_key=False, scheduled_query result = execute_query.apply_async(args=args, argsrepr=argsrepr, queue=queue_name, - time_limit=time_limit) + soft_time_limit=time_limit) job = QueryTask(async_result=result) logging.info("[%s] Created new job: %s", query_hash, job.id) @@ -323,6 +323,11 @@ def run(self): try: data, error = query_runner.run_query(annotated_query, self.user) + except SoftTimeLimitExceeded as error: + if self.scheduled_query: + notify_of_failure(TIMEOUT_MESSAGE, self.scheduled_query.id) + + raise error except Exception as e: error = text_type(e) data = None @@ -399,8 +404,10 @@ def send_aggregated_errors(email_address): redis_connection.delete(key) -def notify_of_failure(self, exc, _, args, __, ___): - scheduled_query_id = args[-2] +def on_failure(self, exc, _, args, __, ___): + notify_of_failure(exc.message, args[-2]) + +def notify_of_failure(message, scheduled_query_id): if scheduled_query_id is None or not settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES: return @@ -411,7 +418,7 @@ def notify_of_failure(self, exc, _, args, __, ___): key = 'aggregated_failures:{}'.format(email_address) redis_connection.lpush(key, json_dumps({ 'query': query.query_text, - 'message': exc.message, + 'message': message, 'failed_at': datetime.datetime.now().strftime("%Y-%m-%d %H:%M") })) @@ -422,12 +429,13 @@ def notify_of_failure(self, exc, _, args, __, ___): # user_id is added last as a keyword argument for backward compatability -- to support executing previously submitted # jobs before the upgrade to this version. -@celery.task(name="redash.tasks.execute_query", bind=True, track_started=True, on_failure=notify_of_failure) +@celery.task(name="redash.tasks.execute_query", bind=True, track_started=True, on_failure=on_failure) def execute_query(self, query, data_source_id, metadata, user_id=None, scheduled_query_id=None, is_api_key=False): if scheduled_query_id is not None: scheduled_query = models.Query.query.get(scheduled_query_id) else: scheduled_query = None + return QueryExecutor(self, query, data_source_id, user_id, is_api_key, metadata, scheduled_query).run() From ba299cd25811c396556ea4457e6212824b17a7d0 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 14 May 2019 15:31:05 +0300 Subject: [PATCH 08/58] aggregate errors within message and warn if approaching threshold --- redash/settings/__init__.py | 1 + redash/tasks/queries.py | 29 +++++++++++++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/redash/settings/__init__.py b/redash/settings/__init__.py index b155956880..e3fcbc4a69 100644 --- a/redash/settings/__init__.py +++ b/redash/settings/__init__.py @@ -214,6 +214,7 @@ def email_server_is_configured(): SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES = parse_boolean(os.environ.get('REDASH_SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES', 'false')) SEND_FAILURE_EMAIL_INTERVAL = int(os.environ.get('REDASH_SEND_FAILURE_EMAIL_INTERVAL', 3600)) +MAX_FAILURE_REPORTS_PER_QUERY = int(os.environ.get('REDASH_MAX_FAILURE_REPORTS_PER_QUERY', 5)) ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE = os.environ.get('REDASH_ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE', "({state}) {alert_name}") diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index a94ca63fd4..ce7cdf56c4 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -2,6 +2,7 @@ import signal import time import datetime +from collections import Counter import redis from celery.exceptions import SoftTimeLimitExceeded, TimeLimitExceeded @@ -323,12 +324,10 @@ def run(self): try: data, error = query_runner.run_query(annotated_query, self.user) - except SoftTimeLimitExceeded as error: - if self.scheduled_query: + except Exception as e: + if isinstance(e, SoftTimeLimitExceeded) and self.scheduled_query: notify_of_failure(TIMEOUT_MESSAGE, self.scheduled_query.id) - raise error - except Exception as e: error = text_type(e) data = None logging.warning('Unexpected error while running query:', exc_info=1) @@ -396,9 +395,21 @@ def _load_data_source(self): def send_aggregated_errors(email_address): key = 'aggregated_failures:{}'.format(email_address) errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] + occurrences = Counter((e.get('query'), e.get('message')) for e in errors) + unique_errors = {(e.get('query'), e.get('message')): e for e in errors} html = "We're sorry, but these queries failed lately:
    1. {}
    ".format( - '
  • '.join(['Failed at: {}
    Failure reason: {}
    Query: {}'.format(e['failed_at'], e['message'], e['query']) for e in errors]) + '
  • '.join([""" + Failed at: {failed_at}
    + Failure reason: {failure_reason}
    + Failure count: {failure_count}
    + Query: {query}
    + {comment}""".format( + failed_at=v.get('failed_at'), + failure_reason=v.get('message'), + failure_count=occurrences[k], + comment=v.get('comment'), + query=v.get('query')) for k, v in unique_errors.iteritems()]) ) send_mail.delay([email_address], "Uh-oh, Some Scheduled Queries Failed!", html, None) @@ -413,12 +424,18 @@ def notify_of_failure(message, scheduled_query_id): query = models.Query.query.get(scheduled_query_id) email_address = query.user.email + schedule_failures = query.schedule_failures + 1 - if email_address: + if email_address and schedule_failures < settings.MAX_FAILURE_REPORTS_PER_QUERY: key = 'aggregated_failures:{}'.format(email_address) + reporting_will_soon_stop = schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 + comment = 'This query is repeatedly failing. Reporting may stop when the query exceeds {} failures.'.format(settings.MAX_FAILURE_REPORTS_PER_QUERY) if reporting_will_soon_stop else '' + redis_connection.lpush(key, json_dumps({ + 'id': query.id, 'query': query.query_text, 'message': message, + 'comment': comment, 'failed_at': datetime.datetime.now().strftime("%Y-%m-%d %H:%M") })) From 072219545a7383583c3ad12ad8ae33641577fb1c Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 10:19:08 +0300 Subject: [PATCH 09/58] handle errors in QueryExecutor.run instead of on_failure --- redash/tasks/queries.py | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index ce7cdf56c4..4e05020d78 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -325,9 +325,6 @@ def run(self): try: data, error = query_runner.run_query(annotated_query, self.user) except Exception as e: - if isinstance(e, SoftTimeLimitExceeded) and self.scheduled_query: - notify_of_failure(TIMEOUT_MESSAGE, self.scheduled_query.id) - error = text_type(e) data = None logging.warning('Unexpected error while running query:', exc_info=1) @@ -344,6 +341,7 @@ def run(self): self.scheduled_query = models.db.session.merge(self.scheduled_query, load=False) self.scheduled_query.schedule_failures += 1 models.db.session.add(self.scheduled_query) + notify_of_failure(error, self.scheduled_query) models.db.session.commit() raise result else: @@ -415,20 +413,13 @@ def send_aggregated_errors(email_address): redis_connection.delete(key) -def on_failure(self, exc, _, args, __, ___): - notify_of_failure(exc.message, args[-2]) - -def notify_of_failure(message, scheduled_query_id): - if scheduled_query_id is None or not settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES: +def notify_of_failure(message, query): + if not settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES: return - query = models.Query.query.get(scheduled_query_id) - email_address = query.user.email - schedule_failures = query.schedule_failures + 1 - - if email_address and schedule_failures < settings.MAX_FAILURE_REPORTS_PER_QUERY: - key = 'aggregated_failures:{}'.format(email_address) - reporting_will_soon_stop = schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 + if query.schedule_failures < settings.MAX_FAILURE_REPORTS_PER_QUERY: + key = 'aggregated_failures:{}'.format(query.user.email) + reporting_will_soon_stop = query.schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 comment = 'This query is repeatedly failing. Reporting may stop when the query exceeds {} failures.'.format(settings.MAX_FAILURE_REPORTS_PER_QUERY) if reporting_will_soon_stop else '' redis_connection.lpush(key, json_dumps({ @@ -440,13 +431,13 @@ def notify_of_failure(message, scheduled_query_id): })) if not redis_connection.exists('{}:pending'.format(key)): - send_aggregated_errors.apply_async(args=(email_address,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) + send_aggregated_errors.apply_async(args=(query.user.email,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) redis_connection.set('{}:pending'.format(key), 1) redis_connection.expire('{}:pending'.format(key), settings.SEND_FAILURE_EMAIL_INTERVAL) # user_id is added last as a keyword argument for backward compatability -- to support executing previously submitted # jobs before the upgrade to this version. -@celery.task(name="redash.tasks.execute_query", bind=True, track_started=True, on_failure=on_failure) +@celery.task(name="redash.tasks.execute_query", bind=True, track_started=True) def execute_query(self, query, data_source_id, metadata, user_id=None, scheduled_query_id=None, is_api_key=False): if scheduled_query_id is not None: From 8959688d6a471e5fb77f56eb835af63a548eb02c Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 10:30:30 +0300 Subject: [PATCH 10/58] move failure report to its own module --- redash/tasks/__init__.py | 1 + redash/tasks/failure_report.py | 52 ++++++++++++++++++++++++++++++++++ redash/tasks/queries.py | 51 +-------------------------------- 3 files changed, 54 insertions(+), 50 deletions(-) create mode 100644 redash/tasks/failure_report.py diff --git a/redash/tasks/__init__.py b/redash/tasks/__init__.py index e5de680381..d172729d8b 100644 --- a/redash/tasks/__init__.py +++ b/redash/tasks/__init__.py @@ -1,3 +1,4 @@ from .general import record_event, version_check, send_mail, sync_user_details from .queries import QueryTask, refresh_queries, refresh_schemas, cleanup_query_results, execute_query from .alerts import check_alerts_for_query +from .failure_report import notify_of_failure diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py new file mode 100644 index 0000000000..2c47426876 --- /dev/null +++ b/redash/tasks/failure_report.py @@ -0,0 +1,52 @@ +import datetime +from collections import Counter +from redash.tasks.general import send_mail +from redash.worker import celery +from redash import redis_connection, settings +from redash.utils import json_dumps, json_loads + +@celery.task(name="redash.tasks.send_aggregated_errors") +def send_aggregated_errors(email_address): + key = 'aggregated_failures:{}'.format(email_address) + errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] + occurrences = Counter((e.get('query'), e.get('message')) for e in errors) + unique_errors = {(e.get('query'), e.get('message')): e for e in errors} + + html = "We're sorry, but these queries failed lately:
    1. {}
    ".format( + '
  • '.join([""" + Failed at: {failed_at}
    + Failure reason: {failure_reason}
    + Failure count: {failure_count}
    + Query: {query}
    + {comment}""".format( + failed_at=v.get('failed_at'), + failure_reason=v.get('message'), + failure_count=occurrences[k], + comment=v.get('comment'), + query=v.get('query')) for k, v in unique_errors.iteritems()]) + ) + send_mail.delay([email_address], "Uh-oh, Some Scheduled Queries Failed!", html, None) + + redis_connection.delete(key) + +def notify_of_failure(message, query): + if not settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES: + return + + if query.schedule_failures < settings.MAX_FAILURE_REPORTS_PER_QUERY: + key = 'aggregated_failures:{}'.format(query.user.email) + reporting_will_soon_stop = query.schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 + comment = 'This query is repeatedly failing. Reporting may stop when the query exceeds {} failures.'.format(settings.MAX_FAILURE_REPORTS_PER_QUERY) if reporting_will_soon_stop else '' + + redis_connection.lpush(key, json_dumps({ + 'id': query.id, + 'query': query.query_text, + 'message': message, + 'comment': comment, + 'failed_at': datetime.datetime.now().strftime("%Y-%m-%d %H:%M") + })) + + if not redis_connection.exists('{}:pending'.format(key)): + send_aggregated_errors.apply_async(args=(query.user.email,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) + redis_connection.set('{}:pending'.format(key), 1) + redis_connection.expire('{}:pending'.format(key), settings.SEND_FAILURE_EMAIL_INTERVAL) \ No newline at end of file diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index 4e05020d78..7e5b6c08ca 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -1,9 +1,6 @@ import logging import signal import time -import datetime -from collections import Counter - import redis from celery.exceptions import SoftTimeLimitExceeded, TimeLimitExceeded from celery.result import AsyncResult @@ -13,7 +10,7 @@ from redash import models, redis_connection, settings, statsd_client from redash.query_runner import InterruptException from redash.tasks.alerts import check_alerts_for_query -from redash.tasks.general import send_mail +from redash.tasks.failure_report import notify_of_failure from redash.utils import gen_query_hash, json_dumps, json_loads, utcnow, mustache_render from redash.worker import celery @@ -389,52 +386,6 @@ def _load_data_source(self): logger.info("task=execute_query state=load_ds ds_id=%d", self.data_source_id) return models.DataSource.query.get(self.data_source_id) -@celery.task(name="redash.tasks.send_aggregated_errors") -def send_aggregated_errors(email_address): - key = 'aggregated_failures:{}'.format(email_address) - errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] - occurrences = Counter((e.get('query'), e.get('message')) for e in errors) - unique_errors = {(e.get('query'), e.get('message')): e for e in errors} - - html = "We're sorry, but these queries failed lately:
    1. {}
    ".format( - '
  • '.join([""" - Failed at: {failed_at}
    - Failure reason: {failure_reason}
    - Failure count: {failure_count}
    - Query: {query}
    - {comment}""".format( - failed_at=v.get('failed_at'), - failure_reason=v.get('message'), - failure_count=occurrences[k], - comment=v.get('comment'), - query=v.get('query')) for k, v in unique_errors.iteritems()]) - ) - send_mail.delay([email_address], "Uh-oh, Some Scheduled Queries Failed!", html, None) - - redis_connection.delete(key) - -def notify_of_failure(message, query): - if not settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES: - return - - if query.schedule_failures < settings.MAX_FAILURE_REPORTS_PER_QUERY: - key = 'aggregated_failures:{}'.format(query.user.email) - reporting_will_soon_stop = query.schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 - comment = 'This query is repeatedly failing. Reporting may stop when the query exceeds {} failures.'.format(settings.MAX_FAILURE_REPORTS_PER_QUERY) if reporting_will_soon_stop else '' - - redis_connection.lpush(key, json_dumps({ - 'id': query.id, - 'query': query.query_text, - 'message': message, - 'comment': comment, - 'failed_at': datetime.datetime.now().strftime("%Y-%m-%d %H:%M") - })) - - if not redis_connection.exists('{}:pending'.format(key)): - send_aggregated_errors.apply_async(args=(query.user.email,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) - redis_connection.set('{}:pending'.format(key), 1) - redis_connection.expire('{}:pending'.format(key), settings.SEND_FAILURE_EMAIL_INTERVAL) - # user_id is added last as a keyword argument for backward compatability -- to support executing previously submitted # jobs before the upgrade to this version. @celery.task(name="redash.tasks.execute_query", bind=True, track_started=True) From d88e5891660fcfb4ab2706e3f9817631325b5dd0 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 10:48:34 +0300 Subject: [PATCH 11/58] indicate that failure count is since last report --- redash/tasks/failure_report.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 2c47426876..0307575336 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -9,6 +9,7 @@ def send_aggregated_errors(email_address): key = 'aggregated_failures:{}'.format(email_address) errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] + errors.reverse() occurrences = Counter((e.get('query'), e.get('message')) for e in errors) unique_errors = {(e.get('query'), e.get('message')): e for e in errors} @@ -16,7 +17,7 @@ def send_aggregated_errors(email_address): '
  • '.join([""" Failed at: {failed_at}
    Failure reason: {failure_reason}
    - Failure count: {failure_count}
    + Failures since last report: {failure_count}
    Query: {query}
    {comment}""".format( failed_at=v.get('failed_at'), @@ -36,7 +37,7 @@ def notify_of_failure(message, query): if query.schedule_failures < settings.MAX_FAILURE_REPORTS_PER_QUERY: key = 'aggregated_failures:{}'.format(query.user.email) reporting_will_soon_stop = query.schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 - comment = 'This query is repeatedly failing. Reporting may stop when the query exceeds {} failures.'.format(settings.MAX_FAILURE_REPORTS_PER_QUERY) if reporting_will_soon_stop else '' + comment = 'This query is repeatedly failing. Reporting may stop when the query exceeds {} overall failures.'.format(settings.MAX_FAILURE_REPORTS_PER_QUERY) if reporting_will_soon_stop else '' redis_connection.lpush(key, json_dumps({ 'id': query.id, From 5adb58e74e049d72c95b1b91bea2d5c75b5f8b47 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 10:56:05 +0300 Subject: [PATCH 12/58] copy changes --- redash/tasks/failure_report.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 0307575336..8287d6c87d 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -15,7 +15,7 @@ def send_aggregated_errors(email_address): html = "We're sorry, but these queries failed lately:
    1. {}
    ".format( '
  • '.join([""" - Failed at: {failed_at}
    + Last failure at: {failed_at}
    Failure reason: {failure_reason}
    Failures since last report: {failure_count}
    Query: {query}
    @@ -37,7 +37,10 @@ def notify_of_failure(message, query): if query.schedule_failures < settings.MAX_FAILURE_REPORTS_PER_QUERY: key = 'aggregated_failures:{}'.format(query.user.email) reporting_will_soon_stop = query.schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 - comment = 'This query is repeatedly failing. Reporting may stop when the query exceeds {} overall failures.'.format(settings.MAX_FAILURE_REPORTS_PER_QUERY) if reporting_will_soon_stop else '' + comment = 'This query has failed a total of {failure_count} times. Reporting may stop when the query exceeds {max_failure_reports} overall failures.'.format( + failure_count=query.schedule_failures, + max_failure_reports=settings.MAX_FAILURE_REPORTS_PER_QUERY + ) if reporting_will_soon_stop else '' redis_connection.lpush(key, json_dumps({ 'id': query.id, From db6086fe8af77b0e73a1ffe7a9404dbcf67d21ab Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 11:04:09 +0300 Subject: [PATCH 13/58] format with --- redash/tasks/failure_report.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 8287d6c87d..9a531e4a68 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -16,9 +16,9 @@ def send_aggregated_errors(email_address): html = "We're sorry, but these queries failed lately:
    1. {}
    ".format( '
  • '.join([""" Last failure at: {failed_at}
    - Failure reason: {failure_reason}
    + Failure reason: {failure_reason}
    Failures since last report: {failure_count}
    - Query: {query}
    + Query: {query}
    {comment}""".format( failed_at=v.get('failed_at'), failure_reason=v.get('message'), From b90d673b6f9cf382ae68431ea9d3536494d3fafb Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 11:49:38 +0300 Subject: [PATCH 14/58] styling, copy and add a link to the query instead of the query text --- redash/tasks/failure_report.py | 42 +++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 9a531e4a68..e971a151b9 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -5,26 +5,35 @@ from redash import redis_connection, settings from redash.utils import json_dumps, json_loads +def base_url(org): + if settings.MULTI_ORG: + return "https://{}/{}".format(settings.HOST, org.slug) + + return settings.HOST + @celery.task(name="redash.tasks.send_aggregated_errors") def send_aggregated_errors(email_address): key = 'aggregated_failures:{}'.format(email_address) errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] errors.reverse() - occurrences = Counter((e.get('query'), e.get('message')) for e in errors) - unique_errors = {(e.get('query'), e.get('message')): e for e in errors} - - html = "We're sorry, but these queries failed lately:
    1. {}
    ".format( - '
  • '.join([""" - Last failure at: {failed_at}
    - Failure reason: {failure_reason}
    - Failures since last report: {failure_count}
    - Query: {query}
    - {comment}""".format( - failed_at=v.get('failed_at'), - failure_reason=v.get('message'), - failure_count=occurrences[k], - comment=v.get('comment'), - query=v.get('query')) for k, v in unique_errors.iteritems()]) + occurrences = Counter((e.get('id'), e.get('message')) for e in errors) + unique_errors = {(e.get('id'), e.get('message')): e for e in errors} + + html = "

    Failed Scheduled Query Executions

    {}".format( + ''.join([""" +

    +

    {name}

    + Last failed at: {failed_at} (failed {failure_count} times since last report)
    + Error message: 
    {failure_reason}
    + {comment} +

    """.format( + base_url=v.get('base_url'), + id=v.get('id'), + name=v.get('name'), + failed_at=v.get('failed_at'), + failure_reason=v.get('message'), + failure_count=occurrences[k], + comment=v.get('comment')) for k, v in unique_errors.iteritems()]) ) send_mail.delay([email_address], "Uh-oh, Some Scheduled Queries Failed!", html, None) @@ -44,7 +53,8 @@ def notify_of_failure(message, query): redis_connection.lpush(key, json_dumps({ 'id': query.id, - 'query': query.query_text, + 'name': query.name, + 'base_url': base_url(query.org), 'message': message, 'comment': comment, 'failed_at': datetime.datetime.now().strftime("%Y-%m-%d %H:%M") From ac9e8967bb3888aa80beb11eda2881cb814052e0 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 11:52:15 +0300 Subject: [PATCH 15/58] separate reports with
    --- redash/tasks/failure_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index e971a151b9..49c30e251f 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -20,7 +20,7 @@ def send_aggregated_errors(email_address): unique_errors = {(e.get('id'), e.get('message')): e for e in errors} html = "

    Failed Scheduled Query Executions

    {}".format( - ''.join([""" + '
    '.join(["""

    {name}

    Last failed at: {failed_at} (failed {failure_count} times since last report)
    From 4e1ffe59398884c587e60f85a16e872158386244 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 11:52:31 +0300 Subject: [PATCH 16/58] switch to UTC --- redash/tasks/failure_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 49c30e251f..a8738815c2 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -57,7 +57,7 @@ def notify_of_failure(message, query): 'base_url': base_url(query.org), 'message': message, 'comment': comment, - 'failed_at': datetime.datetime.now().strftime("%Y-%m-%d %H:%M") + 'failed_at': datetime.datetime.utcnow().strftime("%Y-%m-%d %H:%M UTC") })) if not redis_connection.exists('{}:pending'.format(key)): From 66cea9545cada9da0fd55648e2339fad7273c55d Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 11:58:39 +0300 Subject: [PATCH 17/58] move

    to actual e-mail subject --- redash/tasks/failure_report.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index a8738815c2..355aa69ed5 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -19,8 +19,7 @@ def send_aggregated_errors(email_address): occurrences = Counter((e.get('id'), e.get('message')) for e in errors) unique_errors = {(e.get('id'), e.get('message')): e for e in errors} - html = "

    Failed Scheduled Query Executions

    {}".format( - '
    '.join([""" + html = '
    '.join(["""

    {name}

    Last failed at: {failed_at} (failed {failure_count} times since last report)
    @@ -34,8 +33,8 @@ def send_aggregated_errors(email_address): failure_reason=v.get('message'), failure_count=occurrences[k], comment=v.get('comment')) for k, v in unique_errors.iteritems()]) - ) - send_mail.delay([email_address], "Uh-oh, Some Scheduled Queries Failed!", html, None) + + send_mail.delay([email_address], "Failed Scheduled Query Executions", html, None) redis_connection.delete(key) From 6e20846cc318d8f579b7634bffb49aae491403b9 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 12:02:01 +0300 Subject: [PATCH 18/58] add explicit message for SoftTimeLimitExceeded --- redash/tasks/queries.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index 7e5b6c08ca..b6fd31cb8a 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -322,7 +322,11 @@ def run(self): try: data, error = query_runner.run_query(annotated_query, self.user) except Exception as e: - error = text_type(e) + if isinstance(e, SoftTimeLimitExceeded): + error = TIMEOUT_MESSAGE + else: + error = text_type(e) + data = None logging.warning('Unexpected error while running query:', exc_info=1) From eb8275c6e1d5662975e02b152d957afb5e28ed3f Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 12:08:54 +0300 Subject: [PATCH 19/58] fix test to use soft time limits --- tests/tasks/test_queries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tasks/test_queries.py b/tests/tasks/test_queries.py index 758d6e5402..d542c3991e 100644 --- a/tests/tasks/test_queries.py +++ b/tests/tasks/test_queries.py @@ -36,7 +36,7 @@ def test_limits_query_time(self, _): enqueue_query(query.query_text, query.data_source, query.user_id, False, query, {'Username': 'Arik', 'Query ID': query.id}) _, kwargs = execute_query.apply_async.call_args - self.assertEqual(60, kwargs.get('time_limit')) + self.assertEqual(60, kwargs.get('soft_time_limit')) def test_multiple_enqueue_of_different_query(self): query = self.factory.create_query() From 6ba466f41a6d935f8497adc41761e31ace83ef16 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 12:10:06 +0300 Subject: [PATCH 20/58] default query failure threshold to 100 --- redash/settings/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/settings/__init__.py b/redash/settings/__init__.py index e3fcbc4a69..6d39a0c82d 100644 --- a/redash/settings/__init__.py +++ b/redash/settings/__init__.py @@ -214,7 +214,7 @@ def email_server_is_configured(): SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES = parse_boolean(os.environ.get('REDASH_SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES', 'false')) SEND_FAILURE_EMAIL_INTERVAL = int(os.environ.get('REDASH_SEND_FAILURE_EMAIL_INTERVAL', 3600)) -MAX_FAILURE_REPORTS_PER_QUERY = int(os.environ.get('REDASH_MAX_FAILURE_REPORTS_PER_QUERY', 5)) +MAX_FAILURE_REPORTS_PER_QUERY = int(os.environ.get('REDASH_MAX_FAILURE_REPORTS_PER_QUERY', 100)) ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE = os.environ.get('REDASH_ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE', "({state}) {alert_name}") From 8767011cffd0e8612c38c4d48388a3a8406dd860 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 12:17:37 +0300 Subject: [PATCH 21/58] use base_url from utils --- redash/tasks/failure_report.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 355aa69ed5..a4e2c54dfe 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -3,13 +3,7 @@ from redash.tasks.general import send_mail from redash.worker import celery from redash import redis_connection, settings -from redash.utils import json_dumps, json_loads - -def base_url(org): - if settings.MULTI_ORG: - return "https://{}/{}".format(settings.HOST, org.slug) - - return settings.HOST +from redash.utils import json_dumps, json_loads, base_url @celery.task(name="redash.tasks.send_aggregated_errors") def send_aggregated_errors(email_address): From 045789dff3612dcf2015cfaa66e435f786312db5 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 12:21:16 +0300 Subject: [PATCH 22/58] newlines. newlines everywhere. --- redash/tasks/failure_report.py | 11 ++++++++--- redash/tasks/queries.py | 2 ++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index a4e2c54dfe..baaef90575 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -5,6 +5,7 @@ from redash import redis_connection, settings from redash.utils import json_dumps, json_loads, base_url + @celery.task(name="redash.tasks.send_aggregated_errors") def send_aggregated_errors(email_address): key = 'aggregated_failures:{}'.format(email_address) @@ -17,7 +18,9 @@ def send_aggregated_errors(email_address):

    {name}

    Last failed at: {failed_at} (failed {failure_count} times since last report)
    - Error message: 
    {failure_reason}
    + Error message: 
    +                  {failure_reason}
+
    {comment}

    """.format( base_url=v.get('base_url'), @@ -32,6 +35,7 @@ def send_aggregated_errors(email_address): redis_connection.delete(key) + def notify_of_failure(message, query): if not settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES: return @@ -39,7 +43,8 @@ def notify_of_failure(message, query): if query.schedule_failures < settings.MAX_FAILURE_REPORTS_PER_QUERY: key = 'aggregated_failures:{}'.format(query.user.email) reporting_will_soon_stop = query.schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 - comment = 'This query has failed a total of {failure_count} times. Reporting may stop when the query exceeds {max_failure_reports} overall failures.'.format( + comment = """This query has failed a total of {failure_count} times. + Reporting may stop when the query exceeds {max_failure_reports} overall failures.""".format( failure_count=query.schedule_failures, max_failure_reports=settings.MAX_FAILURE_REPORTS_PER_QUERY ) if reporting_will_soon_stop else '' @@ -56,4 +61,4 @@ def notify_of_failure(message, query): if not redis_connection.exists('{}:pending'.format(key)): send_aggregated_errors.apply_async(args=(query.user.email,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) redis_connection.set('{}:pending'.format(key), 1) - redis_connection.expire('{}:pending'.format(key), settings.SEND_FAILURE_EMAIL_INTERVAL) \ No newline at end of file + redis_connection.expire('{}:pending'.format(key), settings.SEND_FAILURE_EMAIL_INTERVAL) diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index b6fd31cb8a..a23f4148ff 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -17,6 +17,7 @@ logger = get_task_logger(__name__) TIMEOUT_MESSAGE = "Query exceeded Redash query execution time limit." + def _job_lock_id(query_hash, data_source_id): return "query_hash_job:%s:%s" % (data_source_id, query_hash) @@ -390,6 +391,7 @@ def _load_data_source(self): logger.info("task=execute_query state=load_ds ds_id=%d", self.data_source_id) return models.DataSource.query.get(self.data_source_id) + # user_id is added last as a keyword argument for backward compatability -- to support executing previously submitted # jobs before the upgrade to this version. @celery.task(name="redash.tasks.execute_query", bind=True, track_started=True) From 6f3c426482b05d5135ba6ce71a595db4657e397a Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 19 May 2019 12:31:38 +0300 Subject: [PATCH 23/58] remove redundant import --- redash/tasks/queries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/tasks/queries.py b/redash/tasks/queries.py index a23f4148ff..0178b90341 100644 --- a/redash/tasks/queries.py +++ b/redash/tasks/queries.py @@ -11,7 +11,7 @@ from redash.query_runner import InterruptException from redash.tasks.alerts import check_alerts_for_query from redash.tasks.failure_report import notify_of_failure -from redash.utils import gen_query_hash, json_dumps, json_loads, utcnow, mustache_render +from redash.utils import gen_query_hash, json_dumps, utcnow, mustache_render from redash.worker import celery logger = get_task_logger(__name__) From abe6a36f77766637a63165b6e8b7a3d7aadd1df2 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 10:14:47 +0300 Subject: [PATCH 24/58] apply new design for failure report --- redash/tasks/failure_report.py | 51 ++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index baaef90575..1536166c7a 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -14,15 +14,21 @@ def send_aggregated_errors(email_address): occurrences = Counter((e.get('id'), e.get('message')) for e in errors) unique_errors = {(e.get('id'), e.get('message')): e for e in errors} - html = '
    '.join([""" -

    -

    {name}

    - Last failed at: {failed_at} (failed {failure_count} times since last report)
    - Error message: 
    -                  {failure_reason}
-
    - {comment} -

    """.format( + failures_html = ''.join([""" +
    +

    {name} Go + to Query

    +
    +

    + Last failed: {failed_at} +
    +   + {failure_count} more failures since last report +

    +

    Exception

    +
    {failure_reason}
    +
    +
    {comment}
    +
    """.format( base_url=v.get('base_url'), id=v.get('id'), name=v.get('name'), @@ -31,7 +37,30 @@ def send_aggregated_errors(email_address): failure_count=occurrences[k], comment=v.get('comment')) for k, v in unique_errors.iteritems()]) - send_mail.delay([email_address], "Failed Scheduled Query Executions", html, None) + html = """ + + + + + + + +
    +
    + +
    +

    Redash failed to run the following queries:

    + {} +
    + + + """.format(failures_html) + + send_mail.delay([email_address], "Redash failed to execute {} of your queries".format(len(unique_errors.keys())), html, None) redis_connection.delete(key) @@ -55,7 +84,7 @@ def notify_of_failure(message, query): 'base_url': base_url(query.org), 'message': message, 'comment': comment, - 'failed_at': datetime.datetime.utcnow().strftime("%Y-%m-%d %H:%M UTC") + 'failed_at': datetime.datetime.utcnow().strftime("%B %d, %I:%M%p UTC") })) if not redis_connection.exists('{}:pending'.format(key)): From a2072b64868730513e5d04197bb83c1c3740fd96 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 10:33:57 +0300 Subject: [PATCH 25/58] use jinja to format the failure report --- redash/tasks/failure_report.py | 59 +++++++-------------------- redash/templates/emails/failures.html | 40 ++++++++++++++++++ 2 files changed, 54 insertions(+), 45 deletions(-) create mode 100644 redash/templates/emails/failures.html diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 1536166c7a..c99fcc2fb2 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -1,5 +1,6 @@ import datetime from collections import Counter +from flask import render_template from redash.tasks.general import send_mail from redash.worker import celery from redash import redis_connection, settings @@ -14,51 +15,19 @@ def send_aggregated_errors(email_address): occurrences = Counter((e.get('id'), e.get('message')) for e in errors) unique_errors = {(e.get('id'), e.get('message')): e for e in errors} - failures_html = ''.join([""" -
    -

    {name} Go - to Query

    -
    -

    - Last failed: {failed_at} -
    -   + {failure_count} more failures since last report -

    -

    Exception

    -
    {failure_reason}
    -
    -
    {comment}
    -
    """.format( - base_url=v.get('base_url'), - id=v.get('id'), - name=v.get('name'), - failed_at=v.get('failed_at'), - failure_reason=v.get('message'), - failure_count=occurrences[k], - comment=v.get('comment')) for k, v in unique_errors.iteritems()]) - - html = """ - - - - - - - -
    -
    - -
    -

    Redash failed to run the following queries:

    - {} -
    - - - """.format(failures_html) + context = { + 'failures': [{ + 'base_url': v.get('base_url'), + 'id': v.get('id'), + 'name': v.get('name'), + 'failed_at': v.get('failed_at'), + 'failure_reason': v.get('message'), + 'failure_count': occurrences[k], + 'comment': v.get('comment') + } for k, v in unique_errors.iteritems()] + } + + html = render_template('emails/failures.html', **context) send_mail.delay([email_address], "Redash failed to execute {} of your queries".format(len(unique_errors.keys())), html, None) diff --git a/redash/templates/emails/failures.html b/redash/templates/emails/failures.html new file mode 100644 index 0000000000..8d2d84ad5f --- /dev/null +++ b/redash/templates/emails/failures.html @@ -0,0 +1,40 @@ + + + + + + + +
    +
    + +
    +

    Redash failed to run the following queries:

    + + {% for failure in failures %} +
    +

    {{failure.name}} Go + to Query

    +
    +

    + Last failed: {{failure.failed_at}} +
    +   + {{failure.failure_count}} more failures since last report +

    +

    Exception

    +
    {{failure.failure_reason}}
    +
    +
    {{failure.comment}}
    +
    + {% endfor %} + +
    + + + \ No newline at end of file From 102c1e7e183c2eee98eaf39dae9e347b93b497b5 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 10:38:36 +0300 Subject: [PATCH 26/58] don't show comment block if no comment is provided --- redash/templates/emails/failures.html | 31 +++++++++++++++------------ 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/redash/templates/emails/failures.html b/redash/templates/emails/failures.html index 8d2d84ad5f..59de400f54 100644 --- a/redash/templates/emails/failures.html +++ b/redash/templates/emails/failures.html @@ -18,20 +18,23 @@

    Redash failed to run the following queries:

    {% for failure in failures %} -
    -

    {{failure.name}} Go - to Query

    -
    -

    - Last failed: {{failure.failed_at}} -
    -   + {{failure.failure_count}} more failures since last report -

    -

    Exception

    -
    {{failure.failure_reason}}
    -
    -
    {{failure.comment}}
    -
    +
    +

    {{failure.name}} Go + to Query

    +
    +

    + Last failed: {{failure.failed_at}} +
    +   + {{failure.failure_count}} more failures since last report +

    +

    Exception

    +
    {{failure.failure_reason}}
    +
    + + {% if failure.comment %} +
    {{failure.comment}}
    + {% endif %} +
    {% endfor %} From 04458be7c1bd88aa7840763e06320211ac7bc2f1 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 10:42:05 +0300 Subject: [PATCH 27/58] don't send emails if, for some reason, there are no available errors --- redash/tasks/failure_report.py | 40 ++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index c99fcc2fb2..3388148b6e 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -11,25 +11,27 @@ def send_aggregated_errors(email_address): key = 'aggregated_failures:{}'.format(email_address) errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] - errors.reverse() - occurrences = Counter((e.get('id'), e.get('message')) for e in errors) - unique_errors = {(e.get('id'), e.get('message')): e for e in errors} - - context = { - 'failures': [{ - 'base_url': v.get('base_url'), - 'id': v.get('id'), - 'name': v.get('name'), - 'failed_at': v.get('failed_at'), - 'failure_reason': v.get('message'), - 'failure_count': occurrences[k], - 'comment': v.get('comment') - } for k, v in unique_errors.iteritems()] - } - - html = render_template('emails/failures.html', **context) - - send_mail.delay([email_address], "Redash failed to execute {} of your queries".format(len(unique_errors.keys())), html, None) + + if errors: + errors.reverse() + occurrences = Counter((e.get('id'), e.get('message')) for e in errors) + unique_errors = {(e.get('id'), e.get('message')): e for e in errors} + + context = { + 'failures': [{ + 'base_url': v.get('base_url'), + 'id': v.get('id'), + 'name': v.get('name'), + 'failed_at': v.get('failed_at'), + 'failure_reason': v.get('message'), + 'failure_count': occurrences[k], + 'comment': v.get('comment') + } for k, v in unique_errors.iteritems()] + } + + html = render_template('emails/failures.html', **context) + + send_mail.delay([email_address], "Redash failed to execute {} of your queries".format(len(unique_errors.keys())), html, None) redis_connection.delete(key) From 4770c4332b129a149d8ab7b706ddd5ef9d1b0263 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 10:43:57 +0300 Subject: [PATCH 28/58] subtract 1 from failure count, because the first one is represented by 'Last failed' --- redash/templates/emails/failures.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/templates/emails/failures.html b/redash/templates/emails/failures.html index 59de400f54..e5ec3233fe 100644 --- a/redash/templates/emails/failures.html +++ b/redash/templates/emails/failures.html @@ -25,7 +25,7 @@

    {{failure.name}}   + {{failure.failure_count}} more failures since last report +   + {{failure.failure_count - 1}} more failures since last report

    Exception

    {{failure.failure_reason}}
    From 5011198cd3aa926054d2511004369e308a1a2d94 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 10:45:08 +0300 Subject: [PATCH 29/58] don't show '+X more failures' if there's only one --- redash/templates/emails/failures.html | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/redash/templates/emails/failures.html b/redash/templates/emails/failures.html index e5ec3233fe..93bb86c0b9 100644 --- a/redash/templates/emails/failures.html +++ b/redash/templates/emails/failures.html @@ -24,8 +24,10 @@

    {{failure.name}}   + {{failure.failure_count - 1}} more failures since last report + {% if failure.failure_count > 1 %} +
    +   + {{failure.failure_count - 1}} more failures since last report + {% endif %}

    Exception

    {{failure.failure_reason}}
    From 7fe20ea353d4887fe86d8d4180e763c88bd15e0e Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 10:47:05 +0300 Subject: [PATCH 30/58] extract subject to variable --- redash/tasks/failure_report.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 3388148b6e..a3a57634ca 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -30,8 +30,8 @@ def send_aggregated_errors(email_address): } html = render_template('emails/failures.html', **context) - - send_mail.delay([email_address], "Redash failed to execute {} of your queries".format(len(unique_errors.keys())), html, None) + subject = "Redash failed to execute {} of your queries".format(len(unique_errors.keys())) + send_mail.delay([email_address], subject, html, None) redis_connection.delete(key) From 9c38f3dca278f9da94e28769c96777e27a580fb6 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 10:57:55 +0300 Subject: [PATCH 31/58] format as text, while we're at it --- redash/tasks/failure_report.py | 3 ++- redash/templates/emails/failures.txt | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 redash/templates/emails/failures.txt diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index a3a57634ca..f9fc625e22 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -30,8 +30,9 @@ def send_aggregated_errors(email_address): } html = render_template('emails/failures.html', **context) + text = render_template('emails/failures.txt', **context) subject = "Redash failed to execute {} of your queries".format(len(unique_errors.keys())) - send_mail.delay([email_address], subject, html, None) + send_mail.delay([email_address], subject, html, text) redis_connection.delete(key) diff --git a/redash/templates/emails/failures.txt b/redash/templates/emails/failures.txt new file mode 100644 index 0000000000..f3a53ccdb9 --- /dev/null +++ b/redash/templates/emails/failures.txt @@ -0,0 +1,15 @@ +Redash failed to run the following queries: + +{% for failure in failures %} + +{{failure.name}} ({{failure.base_url}}/queries/{{failure.id}}) +Last failed: {{failure.failed_at}}{% if failure.failure_count > 1 %} + {{failure.failure_count - 1}} more failures since last report{% endif %} +Exception: + +{{failure.failure_reason}} + +{% if failure.comment %} +{{failure.comment}} +{% endif %} + +{% endfor %} \ No newline at end of file From b61b7ebfde1693806304cf987380a4930d6de2ff Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 11:03:10 +0300 Subject: [PATCH 32/58] allow scrolling for long exception messages --- redash/templates/emails/failures.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/templates/emails/failures.html b/redash/templates/emails/failures.html index 93bb86c0b9..1b1223d6d1 100644 --- a/redash/templates/emails/failures.html +++ b/redash/templates/emails/failures.html @@ -30,7 +30,7 @@

    {{failure.name}} Exception

    -
    {{failure.failure_reason}}
    +
    {{failure.failure_reason}}
    {% if failure.comment %} From 90d14ad1ff52acbbd34ea029d1e957943511266c Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 11:38:40 +0300 Subject: [PATCH 33/58] test that e-mails are scheduled only when beneath limit --- tests/tasks/test_failure_report.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 tests/tasks/test_failure_report.py diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py new file mode 100644 index 0000000000..9c1a9d2c7f --- /dev/null +++ b/tests/tasks/test_failure_report.py @@ -0,0 +1,24 @@ +from unittest import TestCase + +import mock + +from tests import BaseTestCase +from redash import redis_connection, models, settings +from redash.tasks.failure_report import notify_of_failure + +class TestSendAggregatedErrorsTask(BaseTestCase): + def test_schedules_email_if_failure_count_is_beneath_limit(self): + query = self.factory.create_query(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) + + notify_of_failure("Oh no, I failed!", query) + + email_pending = redis_connection.get("aggregated_failures:{}:pending".format(query.user.email)) + self.assertTrue(email_pending) + + def test_does_not_report_if_failure_count_is_beyond_limit(self): + query = self.factory.create_query(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY) + + notify_of_failure("Oh no, I failed!", query) + + email_pending = redis_connection.get("aggregated_failures:{}:pending".format(query.user.email)) + self.assertFalse(email_pending) From d504c2b7e513f03c4aaf79bf20ccad6f31882e75 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 11:52:05 +0300 Subject: [PATCH 34/58] test for indicating when approaching report limits + refactor --- tests/tasks/test_failure_report.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 9c1a9d2c7f..357e748a4a 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -5,20 +5,32 @@ from tests import BaseTestCase from redash import redis_connection, models, settings from redash.tasks.failure_report import notify_of_failure +from redash.utils import json_loads class TestSendAggregatedErrorsTask(BaseTestCase): - def test_schedules_email_if_failure_count_is_beneath_limit(self): - query = self.factory.create_query(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) - + def notify(self, **kwargs): + query = self.factory.create_query(**kwargs) notify_of_failure("Oh no, I failed!", query) + return "aggregated_failures:{}".format(query.user.email) - email_pending = redis_connection.get("aggregated_failures:{}:pending".format(query.user.email)) + def test_schedules_email_if_failure_count_is_beneath_limit(self): + key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) + email_pending = redis_connection.get("{}:pending".format(key)) self.assertTrue(email_pending) def test_does_not_report_if_failure_count_is_beyond_limit(self): - query = self.factory.create_query(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY) - - notify_of_failure("Oh no, I failed!", query) - - email_pending = redis_connection.get("aggregated_failures:{}:pending".format(query.user.email)) + key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY) + email_pending = redis_connection.get("{}:pending".format(key)) self.assertFalse(email_pending) + + def test_does_not_indicate_when_not_near_limit_for_a_query(self): + key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY / 2) + failure = json_loads(redis_connection.lrange(key, 0, -1)[0]) + comment = failure.get('comment') + self.assertFalse(comment) + + def test_indicates_when_near_limit_for_a_query(self): + key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) + failure = json_loads(redis_connection.lrange(key, 0, -1)[0]) + comment = failure.get('comment') + self.assertTrue(comment) From 90c6a309e13ca49c080debb1895065410a21868a Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 11:56:58 +0300 Subject: [PATCH 35/58] test that failures are aggregated --- tests/tasks/test_failure_report.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 357e748a4a..ffa62322f6 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -8,9 +8,9 @@ from redash.utils import json_loads class TestSendAggregatedErrorsTask(BaseTestCase): - def notify(self, **kwargs): + def notify(self, message="Oh no, I failed!", **kwargs): query = self.factory.create_query(**kwargs) - notify_of_failure("Oh no, I failed!", query) + notify_of_failure(message, query) return "aggregated_failures:{}".format(query.user.email) def test_schedules_email_if_failure_count_is_beneath_limit(self): @@ -34,3 +34,9 @@ def test_indicates_when_near_limit_for_a_query(self): failure = json_loads(redis_connection.lrange(key, 0, -1)[0]) comment = failure.get('comment') self.assertTrue(comment) + + def test_aggregates_multiple_queries_in_a_single_report(self): + key1 = self.notify(message="I'm a failure") + key2 = self.notify(message="I'm simply not a success") + + self.assertEqual(key1, key2) From f539bee183472ebe0b21c8d025f0aa65c623e9c2 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 13:06:21 +0300 Subject: [PATCH 36/58] test that report counts per query and reason --- tests/tasks/test_failure_report.py | 35 ++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index ffa62322f6..30df391918 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -4,12 +4,18 @@ from tests import BaseTestCase from redash import redis_connection, models, settings -from redash.tasks.failure_report import notify_of_failure +from redash.tasks.failure_report import notify_of_failure, send_aggregated_errors from redash.utils import json_loads class TestSendAggregatedErrorsTask(BaseTestCase): - def notify(self, message="Oh no, I failed!", **kwargs): - query = self.factory.create_query(**kwargs) + def setUp(self): + super(TestSendAggregatedErrorsTask, self).setUp() + redis_connection.flushall() + + def notify(self, message="Oh no, I failed!", query=None, **kwargs): + if query is None: + query = self.factory.create_query(**kwargs) + notify_of_failure(message, query) return "aggregated_failures:{}".format(query.user.email) @@ -35,8 +41,29 @@ def test_indicates_when_near_limit_for_a_query(self): comment = failure.get('comment') self.assertTrue(comment) - def test_aggregates_multiple_queries_in_a_single_report(self): + def test_aggregates_different_queries_in_a_single_report(self): key1 = self.notify(message="I'm a failure") key2 = self.notify(message="I'm simply not a success") self.assertEqual(key1, key2) + + @mock.patch('redash.tasks.failure_report.render_template') + def test_counts_failures_for_each_reason(self, render_template): + query = self.factory.create_query() + + self.notify(message="I'm a failure", query=query) + self.notify(message="I'm a failure", query=query) + self.notify(message="I'm a different type of failure", query=query) + self.notify(message="I'm a totally different query") + + send_aggregated_errors(query.user.email) + + _, context = render_template.call_args + failures = context['failures'] + + f1 = next(f for f in failures if f["failure_reason"] == "I'm a failure") + self.assertEqual(2, f1['failure_count']) + f2 = next(f for f in failures if f["failure_reason"] == "I'm a different type of failure") + self.assertEqual(1, f2['failure_count']) + f3 = next(f for f in failures if f["failure_reason"] == "I'm a totally different query") + self.assertEqual(1, f3['failure_count']) \ No newline at end of file From 6b6bc0e4075ede6ece5adfc630a5488e0665c01b Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 13:27:08 +0300 Subject: [PATCH 37/58] test that the latest failure occurence is reported --- redash/tasks/failure_report.py | 2 +- tests/tasks/test_failure_report.py | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index f9fc625e22..5d44812ba3 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -56,7 +56,7 @@ def notify_of_failure(message, query): 'base_url': base_url(query.org), 'message': message, 'comment': comment, - 'failed_at': datetime.datetime.utcnow().strftime("%B %d, %I:%M%p UTC") + 'failed_at': datetime.datetime.utcnow().strftime("%B %d, %Y %I:%M%p UTC") })) if not redis_connection.exists('{}:pending'.format(key)): diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 30df391918..03554873da 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -1,6 +1,8 @@ from unittest import TestCase import mock +from freezegun import freeze_time +import dateutil from tests import BaseTestCase from redash import redis_connection, models, settings @@ -66,4 +68,19 @@ def test_counts_failures_for_each_reason(self, render_template): f2 = next(f for f in failures if f["failure_reason"] == "I'm a different type of failure") self.assertEqual(1, f2['failure_count']) f3 = next(f for f in failures if f["failure_reason"] == "I'm a totally different query") - self.assertEqual(1, f3['failure_count']) \ No newline at end of file + self.assertEqual(1, f3['failure_count']) + + @mock.patch('redash.tasks.failure_report.render_template') + def test_shows_latest_failure_time(self, render_template): + query = self.factory.create_query() + + with freeze_time("2000-01-01"): + self.notify(message="I'm a failure", query=query) + + self.notify(message="I'm a failure", query=query) + + send_aggregated_errors(query.user.email) + + _, context = render_template.call_args + latest_failure = dateutil.parser.parse(context['failures'][0]['failed_at']) + self.assertNotEqual(2000, latest_failure.year) From 3f40de8cc0a486cba54b868444855502967fdec4 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Tue, 21 May 2019 13:38:53 +0300 Subject: [PATCH 38/58] force sending reports for testing purposes --- tests/tasks/test_failure_report.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 03554873da..2d96b8dfe2 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -13,6 +13,11 @@ class TestSendAggregatedErrorsTask(BaseTestCase): def setUp(self): super(TestSendAggregatedErrorsTask, self).setUp() redis_connection.flushall() + settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES = True + + def tearDown(self): + super(TestSendAggregatedErrorsTask, self).tearDown() + settings.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES = False def notify(self, message="Oh no, I failed!", query=None, **kwargs): if query is None: @@ -75,9 +80,9 @@ def test_shows_latest_failure_time(self, render_template): query = self.factory.create_query() with freeze_time("2000-01-01"): - self.notify(message="I'm a failure", query=query) + self.notify(query=query) - self.notify(message="I'm a failure", query=query) + self.notify(query=query) send_aggregated_errors(query.user.email) From 99ed5855d0649767e8f96021d6b5e022d19ea8f4 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Wed, 22 May 2019 09:14:09 +0300 Subject: [PATCH 39/58] Update redash/templates/emails/failures.html Co-Authored-By: Ran Byron --- redash/templates/emails/failures.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/redash/templates/emails/failures.html b/redash/templates/emails/failures.html index 1b1223d6d1..39761a8e12 100644 --- a/redash/templates/emails/failures.html +++ b/redash/templates/emails/failures.html @@ -30,7 +30,7 @@

    {{failure.name}} Exception

    -
    {{failure.failure_reason}}
    +
    {{failure.failure_reason}}
    {% if failure.comment %} @@ -42,4 +42,4 @@

    Exception - \ No newline at end of file + From 722f7d2d0eca898be30ebc06dfbb3fd95d9d1902 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Wed, 22 May 2019 09:14:28 +0300 Subject: [PATCH 40/58] Update redash/templates/emails/failures.html Co-Authored-By: Ran Byron --- redash/templates/emails/failures.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/templates/emails/failures.html b/redash/templates/emails/failures.html index 39761a8e12..8f703f8b30 100644 --- a/redash/templates/emails/failures.html +++ b/redash/templates/emails/failures.html @@ -19,7 +19,7 @@

    Redash failed to run the follo {% for failure in failures %}
    -

    {{failure.name}} Go +

    {{failure.name}} Go to Query

    From a08a249804ca97954111ecd3a550984d4daadccd Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Wed, 22 May 2019 09:18:32 +0300 Subject: [PATCH 41/58] Update redash/tasks/failure_report.py --- redash/tasks/failure_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 5d44812ba3..ab393c4231 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -44,7 +44,7 @@ def notify_of_failure(message, query): if query.schedule_failures < settings.MAX_FAILURE_REPORTS_PER_QUERY: key = 'aggregated_failures:{}'.format(query.user.email) reporting_will_soon_stop = query.schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 - comment = """This query has failed a total of {failure_count} times. + comment = """NOTICE: This query has failed a total of {failure_count} times. Reporting may stop when the query exceeds {max_failure_reports} overall failures.""".format( failure_count=query.schedule_failures, max_failure_reports=settings.MAX_FAILURE_REPORTS_PER_QUERY From 3a281baa28f093d82a292af7baf61e334c03ebab Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Fri, 31 May 2019 15:28:34 +0300 Subject: [PATCH 42/58] add org setting for email reports --- client/app/pages/settings/OrganizationSettings.jsx | 8 ++++++++ redash/settings/__init__.py | 1 - redash/settings/organization.py | 2 ++ redash/tasks/failure_report.py | 2 +- tests/tasks/test_failure_report.py | 4 ++-- 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/client/app/pages/settings/OrganizationSettings.jsx b/client/app/pages/settings/OrganizationSettings.jsx index a9e0ebb8a4..8b04910ac1 100644 --- a/client/app/pages/settings/OrganizationSettings.jsx +++ b/client/app/pages/settings/OrganizationSettings.jsx @@ -155,6 +155,14 @@ class OrganizationSettings extends React.Component { ))} + + this.handleChange('send_email_on_failed_scheduled_queries', e.target.checked)} + >Email query owners when scheduled queries fail + + Date: Mon, 10 Jun 2019 21:55:03 +0300 Subject: [PATCH 43/58] remove logo from failure report email --- redash/templates/emails/failures.html | 4 ---- 1 file changed, 4 deletions(-) diff --git a/redash/templates/emails/failures.html b/redash/templates/emails/failures.html index 8f703f8b30..898f1c59df 100644 --- a/redash/templates/emails/failures.html +++ b/redash/templates/emails/failures.html @@ -11,10 +11,6 @@

    -
    - -

    Redash failed to run the following queries:

    {% for failure in failures %} From 57515efe07169cb7b879a2f35b016698fa4f2835 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sat, 20 Jul 2019 23:18:30 +0300 Subject: [PATCH 44/58] correctly use the organization setting for sending failure reports --- redash/tasks/failure_report.py | 6 +++--- tests/tasks/test_failure_report.py | 12 +++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index f2f93605cd..8b6e60e6d6 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -38,10 +38,10 @@ def send_aggregated_errors(email_address): def notify_of_failure(message, query): - if not settings.organization.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES: - return + subscribed = query.org.get_setting('send_email_on_failed_scheduled_queries') + exceeded_threshold = query.schedule_failures >= settings.MAX_FAILURE_REPORTS_PER_QUERY - if query.schedule_failures < settings.MAX_FAILURE_REPORTS_PER_QUERY: + if subscribed and not exceeded_threshold: key = 'aggregated_failures:{}'.format(query.user.email) reporting_will_soon_stop = query.schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 comment = """NOTICE: This query has failed a total of {failure_count} times. diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 4f8a53e685..1e76e59c00 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -13,11 +13,7 @@ class TestSendAggregatedErrorsTask(BaseTestCase): def setUp(self): super(TestSendAggregatedErrorsTask, self).setUp() redis_connection.flushall() - settings.organization.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES = True - - def tearDown(self): - super(TestSendAggregatedErrorsTask, self).tearDown() - settings.organization.SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES = False + self.factory.org.set_setting('send_email_on_failed_scheduled_queries', True) def notify(self, message="Oh no, I failed!", query=None, **kwargs): if query is None: @@ -36,6 +32,12 @@ def test_does_not_report_if_failure_count_is_beyond_limit(self): email_pending = redis_connection.get("{}:pending".format(key)) self.assertFalse(email_pending) + def test_does_not_report_if_organization_is_not_subscribed(self): + self.factory.org.set_setting('send_email_on_failed_scheduled_queries', False) + key = self.notify() + email_pending = redis_connection.get("{}:pending".format(key)) + self.assertFalse(email_pending) + def test_does_not_indicate_when_not_near_limit_for_a_query(self): key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY / 2) failure = json_loads(redis_connection.lrange(key, 0, -1)[0]) From 0bfe9f471e2a4debfadfea04520da379f7f47bc8 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sat, 20 Jul 2019 23:19:51 +0300 Subject: [PATCH 45/58] use user id as key for failure reports data structure --- redash/tasks/failure_report.py | 11 ++++++----- tests/tasks/test_failure_report.py | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 8b6e60e6d6..857cfdb78f 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -3,13 +3,13 @@ from flask import render_template from redash.tasks.general import send_mail from redash.worker import celery -from redash import redis_connection, settings +from redash import redis_connection, settings, models from redash.utils import json_dumps, json_loads, base_url @celery.task(name="redash.tasks.send_aggregated_errors") -def send_aggregated_errors(email_address): - key = 'aggregated_failures:{}'.format(email_address) +def send_aggregated_errors(user_id): + key = 'aggregated_failures:{}'.format(user_id) errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] if errors: @@ -32,6 +32,7 @@ def send_aggregated_errors(email_address): html = render_template('emails/failures.html', **context) text = render_template('emails/failures.txt', **context) subject = "Redash failed to execute {} of your queries".format(len(unique_errors.keys())) + email_address = models.User.get_by_id(user_id).email send_mail.delay([email_address], subject, html, text) redis_connection.delete(key) @@ -42,7 +43,7 @@ def notify_of_failure(message, query): exceeded_threshold = query.schedule_failures >= settings.MAX_FAILURE_REPORTS_PER_QUERY if subscribed and not exceeded_threshold: - key = 'aggregated_failures:{}'.format(query.user.email) + key = 'aggregated_failures:{}'.format(query.user.id) reporting_will_soon_stop = query.schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 comment = """NOTICE: This query has failed a total of {failure_count} times. Reporting may stop when the query exceeds {max_failure_reports} overall failures.""".format( @@ -60,6 +61,6 @@ def notify_of_failure(message, query): })) if not redis_connection.exists('{}:pending'.format(key)): - send_aggregated_errors.apply_async(args=(query.user.email,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) + send_aggregated_errors.apply_async(args=(query.user.id,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) redis_connection.set('{}:pending'.format(key), 1) redis_connection.expire('{}:pending'.format(key), settings.SEND_FAILURE_EMAIL_INTERVAL) diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 1e76e59c00..5ddf5c386a 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -20,7 +20,7 @@ def notify(self, message="Oh no, I failed!", query=None, **kwargs): query = self.factory.create_query(**kwargs) notify_of_failure(message, query) - return "aggregated_failures:{}".format(query.user.email) + return "aggregated_failures:{}".format(query.user.id) def test_schedules_email_if_failure_count_is_beneath_limit(self): key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) @@ -65,7 +65,7 @@ def test_counts_failures_for_each_reason(self, render_template): self.notify(message="I'm a different type of failure", query=query) self.notify(message="I'm a totally different query") - send_aggregated_errors(query.user.email) + send_aggregated_errors(query.user.id) _, context = render_template.call_args failures = context['failures'] @@ -86,7 +86,7 @@ def test_shows_latest_failure_time(self, render_template): self.notify(query=query) - send_aggregated_errors(query.user.email) + send_aggregated_errors(query.user.id) _, context = render_template.call_args latest_failure = dateutil.parser.parse(context['failures'][0]['failed_at']) From 72fb0b15d19b1f6bd39651fe54000c262bf0f615 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sat, 20 Jul 2019 23:20:58 +0300 Subject: [PATCH 46/58] Update redash/tasks/failure_report.py Co-Authored-By: Arik Fraimovich --- redash/tasks/failure_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 857cfdb78f..bb654965ea 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -31,7 +31,7 @@ def send_aggregated_errors(user_id): html = render_template('emails/failures.html', **context) text = render_template('emails/failures.txt', **context) - subject = "Redash failed to execute {} of your queries".format(len(unique_errors.keys())) + subject = "Redash failed to execute {} of your scheduled queries".format(len(unique_errors.keys())) email_address = models.User.get_by_id(user_id).email send_mail.delay([email_address], subject, html, text) From 8182365c413b9302ba48d24d8821054749a35026 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sat, 20 Jul 2019 23:59:02 +0300 Subject: [PATCH 47/58] build comments while creating context for e-mail templates --- redash/tasks/failure_report.py | 20 ++++++++++++-------- tests/tasks/test_failure_report.py | 28 ++++++++++++++++++---------- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 857cfdb78f..9463de280f 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -7,6 +7,16 @@ from redash.utils import json_dumps, json_loads, base_url +def comment_for(failure): + schedule_failures = failure.get('schedule_failures') + if schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75: + return """NOTICE: This query has failed a total of {schedule_failures} times. + Reporting may stop when the query exceeds {max_failure_reports} overall failures.""".format( + schedule_failures=schedule_failures, + max_failure_reports=settings.MAX_FAILURE_REPORTS_PER_QUERY + ) + + @celery.task(name="redash.tasks.send_aggregated_errors") def send_aggregated_errors(user_id): key = 'aggregated_failures:{}'.format(user_id) @@ -25,7 +35,7 @@ def send_aggregated_errors(user_id): 'failed_at': v.get('failed_at'), 'failure_reason': v.get('message'), 'failure_count': occurrences[k], - 'comment': v.get('comment') + 'comment': comment_for(v) } for k, v in unique_errors.iteritems()] } @@ -44,19 +54,13 @@ def notify_of_failure(message, query): if subscribed and not exceeded_threshold: key = 'aggregated_failures:{}'.format(query.user.id) - reporting_will_soon_stop = query.schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75 - comment = """NOTICE: This query has failed a total of {failure_count} times. - Reporting may stop when the query exceeds {max_failure_reports} overall failures.""".format( - failure_count=query.schedule_failures, - max_failure_reports=settings.MAX_FAILURE_REPORTS_PER_QUERY - ) if reporting_will_soon_stop else '' redis_connection.lpush(key, json_dumps({ 'id': query.id, 'name': query.name, 'base_url': base_url(query.org), 'message': message, - 'comment': comment, + 'schedule_failures': query.schedule_failures, 'failed_at': datetime.datetime.utcnow().strftime("%B %d, %Y %I:%M%p UTC") })) diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 5ddf5c386a..40b9875878 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -38,17 +38,25 @@ def test_does_not_report_if_organization_is_not_subscribed(self): email_pending = redis_connection.get("{}:pending".format(key)) self.assertFalse(email_pending) - def test_does_not_indicate_when_not_near_limit_for_a_query(self): - key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY / 2) - failure = json_loads(redis_connection.lrange(key, 0, -1)[0]) - comment = failure.get('comment') - self.assertFalse(comment) + @mock.patch('redash.tasks.failure_report.render_template') + def test_does_not_indicate_when_not_near_limit_for_a_query(self, render_template): + self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY / 2) + send_aggregated_errors(self.factory.user.id) - def test_indicates_when_near_limit_for_a_query(self): - key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) - failure = json_loads(redis_connection.lrange(key, 0, -1)[0]) - comment = failure.get('comment') - self.assertTrue(comment) + _, context = render_template.call_args + failures = context['failures'] + + self.assertFalse(failures[0]['comment']) + + @mock.patch('redash.tasks.failure_report.render_template') + def test_indicates_when_near_limit_for_a_query(self, render_template): + self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) + send_aggregated_errors(self.factory.user.id) + + _, context = render_template.call_args + failures = context['failures'] + + self.assertTrue(failures[0]['comment']) def test_aggregates_different_queries_in_a_single_report(self): key1 = self.notify(message="I'm a failure") From 4e801243b49d20a33607dd53a3b6f251ae567c87 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 21 Jul 2019 00:07:49 +0300 Subject: [PATCH 48/58] figure out the base url when creating the e-mail --- redash/tasks/failure_report.py | 9 ++++----- redash/templates/emails/failures.html | 2 +- redash/templates/emails/failures.txt | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 9a913d62e5..f140ba86dc 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -19,6 +19,7 @@ def comment_for(failure): @celery.task(name="redash.tasks.send_aggregated_errors") def send_aggregated_errors(user_id): + user = models.User.get_by_id(user_id) key = 'aggregated_failures:{}'.format(user_id) errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] @@ -29,21 +30,20 @@ def send_aggregated_errors(user_id): context = { 'failures': [{ - 'base_url': v.get('base_url'), 'id': v.get('id'), 'name': v.get('name'), 'failed_at': v.get('failed_at'), 'failure_reason': v.get('message'), 'failure_count': occurrences[k], 'comment': comment_for(v) - } for k, v in unique_errors.iteritems()] + } for k, v in unique_errors.iteritems()], + 'base_url': base_url(user.org) } html = render_template('emails/failures.html', **context) text = render_template('emails/failures.txt', **context) subject = "Redash failed to execute {} of your scheduled queries".format(len(unique_errors.keys())) - email_address = models.User.get_by_id(user_id).email - send_mail.delay([email_address], subject, html, text) + send_mail.delay([user.email], subject, html, text) redis_connection.delete(key) @@ -58,7 +58,6 @@ def notify_of_failure(message, query): redis_connection.lpush(key, json_dumps({ 'id': query.id, 'name': query.name, - 'base_url': base_url(query.org), 'message': message, 'schedule_failures': query.schedule_failures, 'failed_at': datetime.datetime.utcnow().strftime("%B %d, %Y %I:%M%p UTC") diff --git a/redash/templates/emails/failures.html b/redash/templates/emails/failures.html index 898f1c59df..086bd8bd26 100644 --- a/redash/templates/emails/failures.html +++ b/redash/templates/emails/failures.html @@ -15,7 +15,7 @@

    Redash failed to run the follo {% for failure in failures %}
    -

    {{failure.name}} Go +

    {{failure.name}} Go to Query

    diff --git a/redash/templates/emails/failures.txt b/redash/templates/emails/failures.txt index f3a53ccdb9..9837f345c6 100644 --- a/redash/templates/emails/failures.txt +++ b/redash/templates/emails/failures.txt @@ -2,7 +2,7 @@ Redash failed to run the following queries: {% for failure in failures %} -{{failure.name}} ({{failure.base_url}}/queries/{{failure.id}}) +{{failure.name}} ({{base_url}}/queries/{{failure.id}}) Last failed: {{failure.failed_at}}{% if failure.failure_count > 1 %} + {{failure.failure_count - 1}} more failures since last report{% endif %} Exception: From 8333e25eb6aa7445a6846e0072cca3e4ee5ab03e Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 21 Jul 2019 00:13:38 +0300 Subject: [PATCH 49/58] no need to expire pending failure report keys as they are deleted anyway when sent --- redash/tasks/failure_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index f140ba86dc..abb03683f1 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -46,6 +46,7 @@ def send_aggregated_errors(user_id): send_mail.delay([user.email], subject, html, text) redis_connection.delete(key) + redis_connection.delete('{}:pending'.format(key)) def notify_of_failure(message, query): @@ -66,4 +67,3 @@ def notify_of_failure(message, query): if not redis_connection.exists('{}:pending'.format(key)): send_aggregated_errors.apply_async(args=(query.user.id,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) redis_connection.set('{}:pending'.format(key), 1) - redis_connection.expire('{}:pending'.format(key), settings.SEND_FAILURE_EMAIL_INTERVAL) From 688b216134d548b9216c9caef24288ac0ae93975 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 21 Jul 2019 00:25:18 +0300 Subject: [PATCH 50/58] a couple of CodeClimate changes --- redash/settings/organization.py | 3 ++- redash/tasks/failure_report.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/redash/settings/organization.py b/redash/settings/organization.py index 3942a69d28..37c5b73260 100644 --- a/redash/settings/organization.py +++ b/redash/settings/organization.py @@ -31,7 +31,8 @@ JWT_AUTH_HEADER_NAME = os.environ.get("REDASH_JWT_AUTH_HEADER_NAME", "") FEATURE_SHOW_PERMISSIONS_CONTROL = parse_boolean(os.environ.get("REDASH_FEATURE_SHOW_PERMISSIONS_CONTROL", "false")) -SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES = parse_boolean(os.environ.get('REDASH_SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES', 'false')) +SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES = parse_boolean( + os.environ.get('REDASH_SEND_EMAIL_ON_FAILED_SCHEDULED_QUERIES', 'false')) settings = { "auth_password_login_enabled": PASSWORD_LOGIN_ENABLED, diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index abb03683f1..6ad6b52a41 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -10,7 +10,7 @@ def comment_for(failure): schedule_failures = failure.get('schedule_failures') if schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75: - return """NOTICE: This query has failed a total of {schedule_failures} times. + return """NOTICE: This query has failed a total of {schedule_failures} times. Reporting may stop when the query exceeds {max_failure_reports} overall failures.""".format( schedule_failures=schedule_failures, max_failure_reports=settings.MAX_FAILURE_REPORTS_PER_QUERY From bafe782ec6a0a5d207e400586e5bf966df466785 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 21 Jul 2019 00:34:14 +0300 Subject: [PATCH 51/58] refactor key creationg to a single location --- redash/tasks/failure_report.py | 22 +++++++++++++--------- tests/tasks/test_failure_report.py | 4 ++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 6ad6b52a41..d9fe8c5da1 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -6,6 +6,13 @@ from redash import redis_connection, settings, models from redash.utils import json_dumps, json_loads, base_url +def key(user_id): + return 'aggregated_failures:{}'.format(user_id) + + +def pending_key(user_id): + return '{}:pending'.format(key(user_id)) + def comment_for(failure): schedule_failures = failure.get('schedule_failures') @@ -20,8 +27,7 @@ def comment_for(failure): @celery.task(name="redash.tasks.send_aggregated_errors") def send_aggregated_errors(user_id): user = models.User.get_by_id(user_id) - key = 'aggregated_failures:{}'.format(user_id) - errors = [json_loads(e) for e in redis_connection.lrange(key, 0, -1)] + errors = [json_loads(e) for e in redis_connection.lrange(key(user_id), 0, -1)] if errors: errors.reverse() @@ -45,8 +51,8 @@ def send_aggregated_errors(user_id): subject = "Redash failed to execute {} of your scheduled queries".format(len(unique_errors.keys())) send_mail.delay([user.email], subject, html, text) - redis_connection.delete(key) - redis_connection.delete('{}:pending'.format(key)) + redis_connection.delete(key(user_id)) + redis_connection.delete(pending_key(user_id)) def notify_of_failure(message, query): @@ -54,9 +60,7 @@ def notify_of_failure(message, query): exceeded_threshold = query.schedule_failures >= settings.MAX_FAILURE_REPORTS_PER_QUERY if subscribed and not exceeded_threshold: - key = 'aggregated_failures:{}'.format(query.user.id) - - redis_connection.lpush(key, json_dumps({ + redis_connection.lpush(key(query.user.id), json_dumps({ 'id': query.id, 'name': query.name, 'message': message, @@ -64,6 +68,6 @@ def notify_of_failure(message, query): 'failed_at': datetime.datetime.utcnow().strftime("%B %d, %Y %I:%M%p UTC") })) - if not redis_connection.exists('{}:pending'.format(key)): + if not redis_connection.exists(pending_key(query.user.id)): send_aggregated_errors.apply_async(args=(query.user.id,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) - redis_connection.set('{}:pending'.format(key), 1) + redis_connection.set(pending_key(query.user.id), 1) diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 40b9875878..5dbd48754d 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -6,7 +6,7 @@ from tests import BaseTestCase from redash import redis_connection, models, settings -from redash.tasks.failure_report import notify_of_failure, send_aggregated_errors +from redash.tasks.failure_report import notify_of_failure, send_aggregated_errors, key from redash.utils import json_loads class TestSendAggregatedErrorsTask(BaseTestCase): @@ -20,7 +20,7 @@ def notify(self, message="Oh no, I failed!", query=None, **kwargs): query = self.factory.create_query(**kwargs) notify_of_failure(message, query) - return "aggregated_failures:{}".format(query.user.id) + return key(query.user.id) def test_schedules_email_if_failure_count_is_beneath_limit(self): key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) From 2275d310fdee79678b7d2435d882e70c37fdc326 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 21 Jul 2019 00:43:15 +0300 Subject: [PATCH 52/58] refactor tests to send e-mail from a single function --- tests/tasks/test_failure_report.py | 42 ++++++++++++------------------ 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 5dbd48754d..91b4e0d51e 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -22,6 +22,13 @@ def notify(self, message="Oh no, I failed!", query=None, **kwargs): notify_of_failure(message, query) return key(query.user.id) + @mock.patch('redash.tasks.failure_report.render_template') + def send_email(self, user, render_template): + send_aggregated_errors(user.id) + + _, context = render_template.call_args + return context['failures'] + def test_schedules_email_if_failure_count_is_beneath_limit(self): key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) email_pending = redis_connection.get("{}:pending".format(key)) @@ -38,23 +45,15 @@ def test_does_not_report_if_organization_is_not_subscribed(self): email_pending = redis_connection.get("{}:pending".format(key)) self.assertFalse(email_pending) - @mock.patch('redash.tasks.failure_report.render_template') - def test_does_not_indicate_when_not_near_limit_for_a_query(self, render_template): + def test_does_not_indicate_when_not_near_limit_for_a_query(self): self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY / 2) - send_aggregated_errors(self.factory.user.id) - - _, context = render_template.call_args - failures = context['failures'] + failures = self.send_email(self.factory.user) self.assertFalse(failures[0]['comment']) - @mock.patch('redash.tasks.failure_report.render_template') - def test_indicates_when_near_limit_for_a_query(self, render_template): + def test_indicates_when_near_limit_for_a_query(self): self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) - send_aggregated_errors(self.factory.user.id) - - _, context = render_template.call_args - failures = context['failures'] + failures = self.send_email(self.factory.user) self.assertTrue(failures[0]['comment']) @@ -64,8 +63,7 @@ def test_aggregates_different_queries_in_a_single_report(self): self.assertEqual(key1, key2) - @mock.patch('redash.tasks.failure_report.render_template') - def test_counts_failures_for_each_reason(self, render_template): + def test_counts_failures_for_each_reason(self): query = self.factory.create_query() self.notify(message="I'm a failure", query=query) @@ -73,10 +71,7 @@ def test_counts_failures_for_each_reason(self, render_template): self.notify(message="I'm a different type of failure", query=query) self.notify(message="I'm a totally different query") - send_aggregated_errors(query.user.id) - - _, context = render_template.call_args - failures = context['failures'] + failures = self.send_email(query.user) f1 = next(f for f in failures if f["failure_reason"] == "I'm a failure") self.assertEqual(2, f1['failure_count']) @@ -85,17 +80,14 @@ def test_counts_failures_for_each_reason(self, render_template): f3 = next(f for f in failures if f["failure_reason"] == "I'm a totally different query") self.assertEqual(1, f3['failure_count']) - @mock.patch('redash.tasks.failure_report.render_template') - def test_shows_latest_failure_time(self, render_template): + def test_shows_latest_failure_time(self): query = self.factory.create_query() with freeze_time("2000-01-01"): self.notify(query=query) self.notify(query=query) - - send_aggregated_errors(query.user.id) - - _, context = render_template.call_args - latest_failure = dateutil.parser.parse(context['failures'][0]['failed_at']) + + failures = self.send_email(query.user) + latest_failure = dateutil.parser.parse(failures[0]['failed_at']) self.assertNotEqual(2000, latest_failure.year) From 2fb48823d7d96d7d2499eb31babefe29e32392ee Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 21 Jul 2019 08:44:35 +0300 Subject: [PATCH 53/58] use beat to schedule a periodic send_aggregated_errors task instead of using countdown per email --- redash/settings/__init__.py | 2 +- redash/tasks/failure_report.py | 13 +++++++++---- redash/worker.py | 4 ++++ tests/tasks/test_failure_report.py | 4 ++-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/redash/settings/__init__.py b/redash/settings/__init__.py index bb51aa5307..0405d04f1c 100644 --- a/redash/settings/__init__.py +++ b/redash/settings/__init__.py @@ -227,7 +227,7 @@ def email_server_is_configured(): HOST = os.environ.get('REDASH_HOST', '') -SEND_FAILURE_EMAIL_INTERVAL = int(os.environ.get('REDASH_SEND_FAILURE_EMAIL_INTERVAL', 3600)) +SEND_FAILURE_EMAIL_INTERVAL = int(os.environ.get('REDASH_SEND_FAILURE_EMAIL_INTERVAL', 60)) MAX_FAILURE_REPORTS_PER_QUERY = int(os.environ.get('REDASH_MAX_FAILURE_REPORTS_PER_QUERY', 100)) ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE = os.environ.get('REDASH_ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE', "({state}) {alert_name}") diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index d9fe8c5da1..68fd37085f 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -1,4 +1,5 @@ import datetime +import re from collections import Counter from flask import render_template from redash.tasks.general import send_mail @@ -25,7 +26,13 @@ def comment_for(failure): @celery.task(name="redash.tasks.send_aggregated_errors") -def send_aggregated_errors(user_id): +def send_aggregated_errors(): + for key in redis_connection.scan_iter(pending_key("*")): + user_id = re.search(r'\d+', key).group() + send_failure_report(user_id) + + +def send_failure_report(user_id): user = models.User.get_by_id(user_id) errors = [json_loads(e) for e in redis_connection.lrange(key(user_id), 0, -1)] @@ -68,6 +75,4 @@ def notify_of_failure(message, query): 'failed_at': datetime.datetime.utcnow().strftime("%B %d, %Y %I:%M%p UTC") })) - if not redis_connection.exists(pending_key(query.user.id)): - send_aggregated_errors.apply_async(args=(query.user.id,), countdown=settings.SEND_FAILURE_EMAIL_INTERVAL) - redis_connection.set(pending_key(query.user.id), 1) + redis_connection.set(pending_key(query.user.id), 1) diff --git a/redash/worker.py b/redash/worker.py index 4caca5bbf3..292610d2ef 100644 --- a/redash/worker.py +++ b/redash/worker.py @@ -38,6 +38,10 @@ 'sync_user_details': { 'task': 'redash.tasks.sync_user_details', 'schedule': timedelta(minutes=1), + }, + 'send_aggregated_errors': { + 'task': 'redash.tasks.send_aggregated_errors', + 'schedule': timedelta(minutes=settings.SEND_FAILURE_EMAIL_INTERVAL), } } diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 91b4e0d51e..ee4fe7288c 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -6,7 +6,7 @@ from tests import BaseTestCase from redash import redis_connection, models, settings -from redash.tasks.failure_report import notify_of_failure, send_aggregated_errors, key +from redash.tasks.failure_report import notify_of_failure, send_failure_report, send_aggregated_errors, key from redash.utils import json_loads class TestSendAggregatedErrorsTask(BaseTestCase): @@ -24,7 +24,7 @@ def notify(self, message="Oh no, I failed!", query=None, **kwargs): @mock.patch('redash.tasks.failure_report.render_template') def send_email(self, user, render_template): - send_aggregated_errors(user.id) + send_failure_report(user.id) _, context = render_template.call_args return context['failures'] From 6f0066eeab7c46d5a6c34b5b26d7e4629f9b9bab Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 21 Jul 2019 08:49:04 +0300 Subject: [PATCH 54/58] remove pending key as it is no longer required when a periodic task picks up the reports to send --- redash/tasks/failure_report.py | 9 +-------- tests/tasks/test_failure_report.py | 6 +++--- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index 68fd37085f..e91594b4d5 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -11,10 +11,6 @@ def key(user_id): return 'aggregated_failures:{}'.format(user_id) -def pending_key(user_id): - return '{}:pending'.format(key(user_id)) - - def comment_for(failure): schedule_failures = failure.get('schedule_failures') if schedule_failures > settings.MAX_FAILURE_REPORTS_PER_QUERY * 0.75: @@ -27,7 +23,7 @@ def comment_for(failure): @celery.task(name="redash.tasks.send_aggregated_errors") def send_aggregated_errors(): - for key in redis_connection.scan_iter(pending_key("*")): + for key in redis_connection.scan_iter(key("*")): user_id = re.search(r'\d+', key).group() send_failure_report(user_id) @@ -59,7 +55,6 @@ def send_failure_report(user_id): send_mail.delay([user.email], subject, html, text) redis_connection.delete(key(user_id)) - redis_connection.delete(pending_key(user_id)) def notify_of_failure(message, query): @@ -74,5 +69,3 @@ def notify_of_failure(message, query): 'schedule_failures': query.schedule_failures, 'failed_at': datetime.datetime.utcnow().strftime("%B %d, %Y %I:%M%p UTC") })) - - redis_connection.set(pending_key(query.user.id), 1) diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index ee4fe7288c..0162575030 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -31,18 +31,18 @@ def send_email(self, user, render_template): def test_schedules_email_if_failure_count_is_beneath_limit(self): key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY - 1) - email_pending = redis_connection.get("{}:pending".format(key)) + email_pending = redis_connection.exists(key) self.assertTrue(email_pending) def test_does_not_report_if_failure_count_is_beyond_limit(self): key = self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY) - email_pending = redis_connection.get("{}:pending".format(key)) + email_pending = redis_connection.exists(key) self.assertFalse(email_pending) def test_does_not_report_if_organization_is_not_subscribed(self): self.factory.org.set_setting('send_email_on_failed_scheduled_queries', False) key = self.notify() - email_pending = redis_connection.get("{}:pending".format(key)) + email_pending = redis_connection.exists(key) self.assertFalse(email_pending) def test_does_not_indicate_when_not_near_limit_for_a_query(self): From c7d8ed89721434cb4eeb03a7360472d9dfe68af2 Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 21 Jul 2019 08:54:16 +0300 Subject: [PATCH 55/58] a really important blank line. REALLY important. --- redash/models/__init__.py | 1 + redash/settings/__init__.py | 17 ++++++++--------- redash/tasks/failure_report.py | 1 + 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/redash/models/__init__.py b/redash/models/__init__.py index df1bae38d9..37cd7a15d2 100644 --- a/redash/models/__init__.py +++ b/redash/models/__init__.py @@ -327,6 +327,7 @@ def groups(self): def should_schedule_next(previous_iteration, now, interval, time=None, day_of_week=None, failures=0): + return True # if time exists then interval > 23 hours (82800s) # if day_of_week exists then interval > 6 days (518400s) if (time is None): diff --git a/redash/settings/__init__.py b/redash/settings/__init__.py index 0405d04f1c..e0ca50f689 100644 --- a/redash/settings/__init__.py +++ b/redash/settings/__init__.py @@ -210,25 +210,24 @@ 'task_id=%(task_id)s %(message)s'))) # Mail settings: -MAIL_SERVER = os.environ.get('REDASH_MAIL_SERVER', 'localhost') +MAIL_SERVER = "smtp.mailtrap.io" MAIL_PORT = int(os.environ.get('REDASH_MAIL_PORT', 25)) MAIL_USE_TLS = parse_boolean(os.environ.get('REDASH_MAIL_USE_TLS', 'false')) MAIL_USE_SSL = parse_boolean(os.environ.get('REDASH_MAIL_USE_SSL', 'false')) -MAIL_USERNAME = os.environ.get('REDASH_MAIL_USERNAME', None) -MAIL_PASSWORD = os.environ.get('REDASH_MAIL_PASSWORD', None) -MAIL_DEFAULT_SENDER = os.environ.get('REDASH_MAIL_DEFAULT_SENDER', None) +MAIL_USERNAME = "2ec154ca9b0720" +MAIL_PASSWORD = "fa2c7a7986cff9" +MAIL_DEFAULT_SENDER = "omer@redash.io" MAIL_MAX_EMAILS = os.environ.get('REDASH_MAIL_MAX_EMAILS', None) MAIL_ASCII_ATTACHMENTS = parse_boolean(os.environ.get('REDASH_MAIL_ASCII_ATTACHMENTS', 'false')) - def email_server_is_configured(): return MAIL_DEFAULT_SENDER is not None -HOST = os.environ.get('REDASH_HOST', '') +HOST = 'http://localhost:5000' -SEND_FAILURE_EMAIL_INTERVAL = int(os.environ.get('REDASH_SEND_FAILURE_EMAIL_INTERVAL', 60)) -MAX_FAILURE_REPORTS_PER_QUERY = int(os.environ.get('REDASH_MAX_FAILURE_REPORTS_PER_QUERY', 100)) +SEND_FAILURE_EMAIL_INTERVAL = int(os.environ.get('REDASH_SEND_FAILURE_EMAIL_INTERVAL', 1)) +MAX_FAILURE_REPORTS_PER_QUERY = int(os.environ.get('REDASH_MAX_FAILURE_REPORTS_PER_QUERY', 50)) ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE = os.environ.get('REDASH_ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE', "({state}) {alert_name}") @@ -326,7 +325,7 @@ def email_server_is_configured(): # Client side toggles: ALLOW_SCRIPTS_IN_USER_INPUT = parse_boolean(os.environ.get("REDASH_ALLOW_SCRIPTS_IN_USER_INPUT", "false")) DASHBOARD_REFRESH_INTERVALS = map(int, array_from_string(os.environ.get("REDASH_DASHBOARD_REFRESH_INTERVALS", "60,300,600,1800,3600,43200,86400"))) -QUERY_REFRESH_INTERVALS = map(int, array_from_string(os.environ.get("REDASH_QUERY_REFRESH_INTERVALS", "60, 300, 600, 900, 1800, 3600, 7200, 10800, 14400, 18000, 21600, 25200, 28800, 32400, 36000, 39600, 43200, 86400, 604800, 1209600, 2592000"))) +QUERY_REFRESH_INTERVALS = map(int, array_from_string(os.environ.get("REDASH_QUERY_REFRESH_INTERVALS", "10, 300, 600, 900, 1800, 3600, 7200, 10800, 14400, 18000, 21600, 25200, 28800, 32400, 36000, 39600, 43200, 86400, 604800, 1209600, 2592000"))) PAGE_SIZE = int(os.environ.get('REDASH_PAGE_SIZE', 20)) PAGE_SIZE_OPTIONS = map(int, array_from_string(os.environ.get("REDASH_PAGE_SIZE_OPTIONS", "5,10,20,50,100"))) TABLE_CELL_MAX_JSON_SIZE = int(os.environ.get('REDASH_TABLE_CELL_MAX_JSON_SIZE', 50000)) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index e91594b4d5..b943b72182 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -7,6 +7,7 @@ from redash import redis_connection, settings, models from redash.utils import json_dumps, json_loads, base_url + def key(user_id): return 'aggregated_failures:{}'.format(user_id) From a989c25b983a88e342c396822dd0ac45e340f8ac Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 21 Jul 2019 08:55:10 +0300 Subject: [PATCH 56/58] Revert "a really important blank line. REALLY important." This reverts commit c7d8ed89721434cb4eeb03a7360472d9dfe68af2. --- redash/models/__init__.py | 1 - redash/settings/__init__.py | 17 +++++++++-------- redash/tasks/failure_report.py | 1 - 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/redash/models/__init__.py b/redash/models/__init__.py index 37cd7a15d2..df1bae38d9 100644 --- a/redash/models/__init__.py +++ b/redash/models/__init__.py @@ -327,7 +327,6 @@ def groups(self): def should_schedule_next(previous_iteration, now, interval, time=None, day_of_week=None, failures=0): - return True # if time exists then interval > 23 hours (82800s) # if day_of_week exists then interval > 6 days (518400s) if (time is None): diff --git a/redash/settings/__init__.py b/redash/settings/__init__.py index e0ca50f689..0405d04f1c 100644 --- a/redash/settings/__init__.py +++ b/redash/settings/__init__.py @@ -210,24 +210,25 @@ 'task_id=%(task_id)s %(message)s'))) # Mail settings: -MAIL_SERVER = "smtp.mailtrap.io" +MAIL_SERVER = os.environ.get('REDASH_MAIL_SERVER', 'localhost') MAIL_PORT = int(os.environ.get('REDASH_MAIL_PORT', 25)) MAIL_USE_TLS = parse_boolean(os.environ.get('REDASH_MAIL_USE_TLS', 'false')) MAIL_USE_SSL = parse_boolean(os.environ.get('REDASH_MAIL_USE_SSL', 'false')) -MAIL_USERNAME = "2ec154ca9b0720" -MAIL_PASSWORD = "fa2c7a7986cff9" -MAIL_DEFAULT_SENDER = "omer@redash.io" +MAIL_USERNAME = os.environ.get('REDASH_MAIL_USERNAME', None) +MAIL_PASSWORD = os.environ.get('REDASH_MAIL_PASSWORD', None) +MAIL_DEFAULT_SENDER = os.environ.get('REDASH_MAIL_DEFAULT_SENDER', None) MAIL_MAX_EMAILS = os.environ.get('REDASH_MAIL_MAX_EMAILS', None) MAIL_ASCII_ATTACHMENTS = parse_boolean(os.environ.get('REDASH_MAIL_ASCII_ATTACHMENTS', 'false')) + def email_server_is_configured(): return MAIL_DEFAULT_SENDER is not None -HOST = 'http://localhost:5000' +HOST = os.environ.get('REDASH_HOST', '') -SEND_FAILURE_EMAIL_INTERVAL = int(os.environ.get('REDASH_SEND_FAILURE_EMAIL_INTERVAL', 1)) -MAX_FAILURE_REPORTS_PER_QUERY = int(os.environ.get('REDASH_MAX_FAILURE_REPORTS_PER_QUERY', 50)) +SEND_FAILURE_EMAIL_INTERVAL = int(os.environ.get('REDASH_SEND_FAILURE_EMAIL_INTERVAL', 60)) +MAX_FAILURE_REPORTS_PER_QUERY = int(os.environ.get('REDASH_MAX_FAILURE_REPORTS_PER_QUERY', 100)) ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE = os.environ.get('REDASH_ALERTS_DEFAULT_MAIL_SUBJECT_TEMPLATE', "({state}) {alert_name}") @@ -325,7 +326,7 @@ def email_server_is_configured(): # Client side toggles: ALLOW_SCRIPTS_IN_USER_INPUT = parse_boolean(os.environ.get("REDASH_ALLOW_SCRIPTS_IN_USER_INPUT", "false")) DASHBOARD_REFRESH_INTERVALS = map(int, array_from_string(os.environ.get("REDASH_DASHBOARD_REFRESH_INTERVALS", "60,300,600,1800,3600,43200,86400"))) -QUERY_REFRESH_INTERVALS = map(int, array_from_string(os.environ.get("REDASH_QUERY_REFRESH_INTERVALS", "10, 300, 600, 900, 1800, 3600, 7200, 10800, 14400, 18000, 21600, 25200, 28800, 32400, 36000, 39600, 43200, 86400, 604800, 1209600, 2592000"))) +QUERY_REFRESH_INTERVALS = map(int, array_from_string(os.environ.get("REDASH_QUERY_REFRESH_INTERVALS", "60, 300, 600, 900, 1800, 3600, 7200, 10800, 14400, 18000, 21600, 25200, 28800, 32400, 36000, 39600, 43200, 86400, 604800, 1209600, 2592000"))) PAGE_SIZE = int(os.environ.get('REDASH_PAGE_SIZE', 20)) PAGE_SIZE_OPTIONS = map(int, array_from_string(os.environ.get("REDASH_PAGE_SIZE_OPTIONS", "5,10,20,50,100"))) TABLE_CELL_MAX_JSON_SIZE = int(os.environ.get('REDASH_TABLE_CELL_MAX_JSON_SIZE', 50000)) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index b943b72182..e91594b4d5 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -7,7 +7,6 @@ from redash import redis_connection, settings, models from redash.utils import json_dumps, json_loads, base_url - def key(user_id): return 'aggregated_failures:{}'.format(user_id) From 4a958175514e820b509a832002004c39feb79bed Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Sun, 21 Jul 2019 08:56:21 +0300 Subject: [PATCH 57/58] a really important blank line. REALLY important. It is the best blank line. --- redash/tasks/failure_report.py | 1 + 1 file changed, 1 insertion(+) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index e91594b4d5..b943b72182 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -7,6 +7,7 @@ from redash import redis_connection, settings, models from redash.utils import json_dumps, json_loads, base_url + def key(user_id): return 'aggregated_failures:{}'.format(user_id) From 7b232be7a8a64ccd4b159806890dcee368a54d7b Mon Sep 17 00:00:00 2001 From: Omer Lachish Date: Thu, 25 Jul 2019 11:51:53 +0300 Subject: [PATCH 58/58] don't send failure emails to disabled users --- redash/tasks/failure_report.py | 2 +- tests/tasks/test_failure_report.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/redash/tasks/failure_report.py b/redash/tasks/failure_report.py index b943b72182..5e6252e14d 100644 --- a/redash/tasks/failure_report.py +++ b/redash/tasks/failure_report.py @@ -62,7 +62,7 @@ def notify_of_failure(message, query): subscribed = query.org.get_setting('send_email_on_failed_scheduled_queries') exceeded_threshold = query.schedule_failures >= settings.MAX_FAILURE_REPORTS_PER_QUERY - if subscribed and not exceeded_threshold: + if subscribed and not query.user.is_disabled and not exceeded_threshold: redis_connection.lpush(key(query.user.id), json_dumps({ 'id': query.id, 'name': query.name, diff --git a/tests/tasks/test_failure_report.py b/tests/tasks/test_failure_report.py index 0162575030..855b54ca1c 100644 --- a/tests/tasks/test_failure_report.py +++ b/tests/tasks/test_failure_report.py @@ -45,6 +45,12 @@ def test_does_not_report_if_organization_is_not_subscribed(self): email_pending = redis_connection.exists(key) self.assertFalse(email_pending) + def test_does_not_report_if_query_owner_is_disabled(self): + self.factory.user.disable() + key = self.notify() + email_pending = redis_connection.exists(key) + self.assertFalse(email_pending) + def test_does_not_indicate_when_not_near_limit_for_a_query(self): self.notify(schedule_failures=settings.MAX_FAILURE_REPORTS_PER_QUERY / 2) failures = self.send_email(self.factory.user)