Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

fix: cache warmup solution non legacy charts. #23012

Merged
merged 5 commits into from
Feb 14, 2023
Merged

fix: cache warmup solution non legacy charts. #23012

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4ed6c14
querycontext
Feb 6, 2023
369ff67
Removed constants charts array. handled the execution based on review…
Feb 13, 2023
df2fe9a
Exporting query context with chart for cache-warmup feature to work f…
Feb 14, 2023
a6e561c
Merge branch 'master' into cache_warmup-solution-non-legacy-charts
villebro Feb 14, 2023
4a375e7
fix tests and remove redundant test
villebro Feb 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions superset/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,32 @@ class CacheRegion(str, Enum):
DEFAULT = "default"
DATA = "data"
THUMBNAIL = "thumbnail"


legacy_charts = [
"line",
"dist_bar",
"area",
"time_table",
"histogram",
"time_pivot",
"heatmap",
"dual_line",
"line_multi",
"treemap",
"sunburst",
"sankey",
"mapbox",
"rose",
"bubble",
"horizon",
"compare",
"partition",
"event_flow",
"world_map",
"paired_ttest",
"para",
"bullet",
"chord",
"pivot_table",
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a more dynamic approach would be to check if there's a query context for a given slice. If there is, assume it's the new type, and if there isn't, assume it's legacy.

Copy link
Contributor Author

@dheeraj-jaiswal-lowes dheeraj-jaiswal-lowes Feb 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@villebro Right. But there are scenarios where query context are not found even for new charts. Like When you Import a chart query context is not found. it is handled in Alert and reports using a function called "update_query_context". I took this approach thinking that number of legacy charts going to constant anyway.
I found no other way to differentiate between charts. Please suggest if you have any other idea.

55 changes: 35 additions & 20 deletions superset/views/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
)
from superset.charts.commands.exceptions import ChartNotFoundError
from superset.charts.dao import ChartDAO
from superset.charts.data.commands.get_data_command import ChartDataCommand
from superset.common.chart_data import ChartDataResultFormat, ChartDataResultType
from superset.common.db_query_status import QueryStatus
from superset.connectors.base.models import BaseDatasource
Expand All @@ -65,7 +66,7 @@
SqlMetric,
TableColumn,
)
from superset.constants import QUERY_EARLY_CANCEL_KEY
from superset.constants import legacy_charts, QUERY_EARLY_CANCEL_KEY
from superset.dashboards.commands.importers.v0 import ImportDashboardsCommand
from superset.dashboards.dao import DashboardDAO
from superset.dashboards.permalink.commands.get import GetDashboardPermalinkCommand
Expand Down Expand Up @@ -1740,28 +1741,42 @@ def warm_up_cache( # pylint: disable=too-many-locals,no-self-use

for slc in slices:
try:
form_data = get_form_data(slc.id, use_slice_data=True)[0]
if dashboard_id:
form_data["extra_filters"] = (
json.loads(extra_filters)
if extra_filters
else get_dashboard_extra_filters(slc.id, dashboard_id)
)
if slc.viz_type in legacy_charts:

form_data = get_form_data(slc.id, use_slice_data=True)[0]
if dashboard_id:
form_data["extra_filters"] = (
json.loads(extra_filters)
if extra_filters
else get_dashboard_extra_filters(slc.id, dashboard_id)
)

if not slc.datasource:
raise Exception("Slice's datasource does not exist")
if not slc.datasource:
raise Exception("Slice's datasource does not exist")

obj = get_viz(
datasource_type=slc.datasource.type,
datasource_id=slc.datasource.id,
form_data=form_data,
force=True,
)
obj = get_viz(
datasource_type=slc.datasource.type,
datasource_id=slc.datasource.id,
form_data=form_data,
force=True,
)

# pylint: disable=assigning-non-slot
g.form_data = form_data
payload = obj.get_payload()
delattr(g, "form_data")
else:
query_context = slc.get_query_context()
if not query_context:
raise Exception(
f"Query conext not found for the chart {slc.slice_name}. Please load the chart and save it manually"
)
if query_context != None:
query_context.force = True
command = ChartDataCommand(query_context)
command.validate()
payload = command.run()

# pylint: disable=assigning-non-slot
g.form_data = form_data
payload = obj.get_payload()
delattr(g, "form_data")
error = payload["errors"] or None
status = payload["status"]
except Exception as ex: # pylint: disable=broad-except
Expand Down