Skip to content

Commit

Permalink
Attempt at fixing scheduled parameterized queries
Browse files Browse the repository at this point in the history
- possible logical error in scheduler condition in should_schedule_next
  function - should apply to queries which next_iteration is after 'now'
- generating query_hash on query expanded with parameters values to
  match query_result with queries and update retrieved_at value
- moved apply_default_parameters in utils
  • Loading branch information
Rémi Laurent committed May 25, 2023
1 parent 962f13e commit 02b9c66
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 27 deletions.
7 changes: 4 additions & 3 deletions redash/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
mustache_render,
base_url,
sentry,
gen_query_hash)
gen_query_hash,
apply_default_parameters)
from redash.utils.configuration import ConfigurationContainer
from redash.models.parameterized_query import ParameterizedQuery

Expand Down Expand Up @@ -446,7 +447,7 @@ def should_schedule_next(
next_iteration += datetime.timedelta(minutes=2 ** failures)
except OverflowError:
return False
return now > next_iteration
return next_iteration > now


@gfk_type
Expand Down Expand Up @@ -868,7 +869,7 @@ def dashboard_api_keys(self):
def update_query_hash(self):
should_apply_auto_limit = self.options.get("apply_auto_limit", False) if self.options else False
query_runner = self.data_source.query_runner if self.data_source else BaseQueryRunner({})
self.query_hash = query_runner.gen_query_hash(self.query_text, should_apply_auto_limit)
self.query_hash = query_runner.gen_query_hash(apply_default_parameters(self), should_apply_auto_limit)


@listens_for(Query, "before_insert")
Expand Down
28 changes: 4 additions & 24 deletions redash/tasks/queries/maintenance.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
QueryDetachedFromDataSourceError,
)
from redash.tasks.failure_report import track_failure
from redash.utils import json_dumps, sentry
from redash.utils import json_dumps, sentry, apply_default_parameters


from redash.worker import job, get_job_logger
from redash.monitor import rq_job_ids

Expand Down Expand Up @@ -50,28 +52,6 @@ def _should_refresh_query(query):
return True


def _apply_default_parameters(query):
parameters = {p["name"]: p.get("value") for p in query.parameters}
if any(parameters):
try:
return query.parameterized.apply(parameters).query
except InvalidParameterError as e:
error = u"Skipping refresh of {} because of invalid parameters: {}".format(
query.id, str(e)
)
track_failure(query, error)
raise
except QueryDetachedFromDataSourceError as e:
error = (
"Skipping refresh of {} because a related dropdown "
"query ({}) is unattached to any datasource."
).format(query.id, e.query_id)
track_failure(query, error)
raise
else:
return query.query_text


class RefreshQueriesError(Exception):
pass

Expand All @@ -91,7 +71,7 @@ def refresh_queries():
continue

try:
query_text = _apply_default_parameters(query)
query_text = apply_default_parameters(query)
query_text = _apply_auto_limit(query_text, query)
enqueue_query(
query_text,
Expand Down
21 changes: 21 additions & 0 deletions redash/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,24 @@ def render_template(path, context):
function decorated with the `context_processor` decorator, which is not explicitly required for rendering purposes.
"""
return current_app.jinja_env.get_template(path).render(**context)

def apply_default_parameters(query):
parameters = {p["name"]: p.get("value") for p in query.parameters}
if any(parameters):
try:
return query.parameterized.apply(parameters).query
except InvalidParameterError as e:
error = u"Skipping refresh of {} because of invalid parameters: {}".format(
query.id, str(e)
)
track_failure(query, error)
raise
except QueryDetachedFromDataSourceError as e:
error = (
"Skipping refresh of {} because a related dropdown "
"query ({}) is unattached to any datasource."
).format(query.id, e.query_id)
track_failure(query, error)
raise
else:
return query.query_text

0 comments on commit 02b9c66

Please sign in to comment.