From 05b97fff4dd56a480405b4ada65de712b3028ecc Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Tue, 13 Sep 2022 15:56:04 +0200 Subject: [PATCH] fix(cache): respect default cache timeout on v1 chart data requests (#21441) --- .../workflows/superset-python-unittest.yml | 2 +- scripts/python_tests.sh | 2 +- superset/charts/schemas.py | 8 ++- superset/common/query_context_processor.py | 6 ++ .../charts/data/api_tests.py | 63 ++++++++++++++++++- 5 files changed, 77 insertions(+), 4 deletions(-) diff --git a/.github/workflows/superset-python-unittest.yml b/.github/workflows/superset-python-unittest.yml index 64db4d3e649af..1ff07375d4727 100644 --- a/.github/workflows/superset-python-unittest.yml +++ b/.github/workflows/superset-python-unittest.yml @@ -37,7 +37,7 @@ jobs: python-version: ${{ matrix.python-version }} cache: 'pip' cache-dependency-path: 'requirements/testing.txt' -# TODO: separated requiermentes.txt file just for unit tests +# TODO: separated requirements.txt file just for unit tests - name: Install dependencies if: steps.check.outcome == 'failure' uses: ./.github/actions/cached-dependencies diff --git a/scripts/python_tests.sh b/scripts/python_tests.sh index 0554f8ca65ad9..6491a3f6f9d46 100755 --- a/scripts/python_tests.sh +++ b/scripts/python_tests.sh @@ -32,4 +32,4 @@ superset init echo "Running tests" -pytest --durations-min=2 --maxfail=1 --cov-report= --cov=superset "$@" +pytest --durations-min=2 --maxfail=1 --cov-report= --cov=superset ./tests/integration_tests "$@" diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index c06768d127b55..ffd64e8bb023a 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -1194,6 +1194,12 @@ class ChartDataQueryContextSchema(Schema): query_context_factory: Optional[QueryContextFactory] = None datasource = fields.Nested(ChartDataDatasourceSchema) queries = fields.List(fields.Nested(ChartDataQueryObjectSchema)) + custom_cache_timeout = fields.Integer( + description="Override the default cache timeout", + required=False, + allow_none=True, + ) + force = fields.Boolean( description="Should the queries be forced to load from the source. " "Default: `false`", @@ -1255,7 +1261,7 @@ class ChartDataResponseResult(Schema): ) cache_timeout = fields.Integer( description="Cache timeout in following order: custom timeout, datasource " - "timeout, default config timeout.", + "timeout, cache default timeout, config default cache timeout.", required=True, allow_none=True, ) diff --git a/superset/common/query_context_processor.py b/superset/common/query_context_processor.py index b253caa6b980d..ae3202981b906 100644 --- a/superset/common/query_context_processor.py +++ b/superset/common/query_context_processor.py @@ -434,6 +434,12 @@ def get_cache_timeout(self) -> int: cache_timeout_rv = self._query_context.get_cache_timeout() if cache_timeout_rv: return cache_timeout_rv + if ( + data_cache_timeout := config["DATA_CACHE_CONFIG"].get( + "CACHE_DEFAULT_TIMEOUT" + ) + ) is not None: + return data_cache_timeout return config["CACHE_DEFAULT_TIMEOUT"] def cache_key(self, **extra: Any) -> str: diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py index 67b9826d26f39..73d33cd793b76 100644 --- a/tests/integration_tests/charts/data/api_tests.py +++ b/tests/integration_tests/charts/data/api_tests.py @@ -21,7 +21,7 @@ import copy from datetime import datetime from io import BytesIO -from typing import Optional +from typing import Any, Dict, Optional from unittest import mock from zipfile import ZipFile @@ -915,3 +915,64 @@ def test_chart_data_with_adhoc_column(self): unique_genders = {row["male_or_female"] for row in data} assert unique_genders == {"male", "female"} assert result["applied_filters"] == [{"column": "male_or_female"}] + + +@pytest.fixture() +def physical_query_context(physical_dataset) -> Dict[str, Any]: + return { + "datasource": { + "type": physical_dataset.type, + "id": physical_dataset.id, + }, + "queries": [ + { + "columns": ["col1"], + "metrics": ["count"], + "orderby": [["col1", True]], + } + ], + "result_type": ChartDataResultType.FULL, + "force": True, + } + + +@mock.patch( + "superset.common.query_context_processor.config", + { + **app.config, + "CACHE_DEFAULT_TIMEOUT": 1234, + "DATA_CACHE_CONFIG": { + **app.config["DATA_CACHE_CONFIG"], + "CACHE_DEFAULT_TIMEOUT": None, + }, + }, +) +def test_cache_default_timeout(test_client, login_as_admin, physical_query_context): + rv = test_client.post(CHART_DATA_URI, json=physical_query_context) + assert rv.json["result"][0]["cache_timeout"] == 1234 + + +def test_custom_cache_timeout(test_client, login_as_admin, physical_query_context): + physical_query_context["custom_cache_timeout"] = 5678 + rv = test_client.post(CHART_DATA_URI, json=physical_query_context) + assert rv.json["result"][0]["cache_timeout"] == 5678 + + +@mock.patch( + "superset.common.query_context_processor.config", + { + **app.config, + "CACHE_DEFAULT_TIMEOUT": 100000, + "DATA_CACHE_CONFIG": { + **app.config["DATA_CACHE_CONFIG"], + "CACHE_DEFAULT_TIMEOUT": 3456, + }, + }, +) +def test_data_cache_default_timeout( + test_client, + login_as_admin, + physical_query_context, +): + rv = test_client.post(CHART_DATA_URI, json=physical_query_context) + assert rv.json["result"][0]["cache_timeout"] == 3456