Skip to content

Commit

Permalink
fix(cache): respect default cache timeout on v1 chart data requests (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
villebro authored Sep 13, 2022
1 parent eb4ba5b commit 05b97ff
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 4 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/superset-python-unittest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion scripts/python_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 "$@"
8 changes: 7 additions & 1 deletion superset/charts/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`",
Expand Down Expand Up @@ -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,
)
Expand Down
6 changes: 6 additions & 0 deletions superset/common/query_context_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
63 changes: 62 additions & 1 deletion tests/integration_tests/charts/data/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

0 comments on commit 05b97ff

Please sign in to comment.