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

chore: Remove obsolete legacy visualizations #24694

Merged
merged 1 commit into from
Jul 18, 2023
Merged

chore: Remove obsolete legacy visualizations #24694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
65 changes: 50 additions & 15 deletions superset/common/query_context_processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@
from superset.common.query_actions import get_query_results
from superset.common.utils import dataframe_utils
from superset.common.utils.query_cache_manager import QueryCacheManager
from superset.common.utils.time_range_utils import get_since_until_from_query_object
from superset.common.utils.time_range_utils import (
get_since_until_from_query_object,
get_since_until_from_time_range,
)
from superset.connectors.base.models import BaseDatasource
from superset.constants import CacheRegion, TimeGrain
from superset.daos.annotation import AnnotationLayerDAO
Expand Down Expand Up @@ -64,6 +67,7 @@
from superset.utils.date_parser import get_past_or_future, normalize_time_delta
from superset.utils.pandas_postprocessing.utils import unescape_separator
from superset.views.utils import get_viz
from superset.viz import viz_types

if TYPE_CHECKING:
from superset.common.query_context import QueryContext
Expand Down Expand Up @@ -685,22 +689,53 @@ def get_native_annotation_data(query_obj: QueryObject) -> dict[str, Any]:
def get_viz_annotation_data(
annotation_layer: dict[str, Any], force: bool
) -> dict[str, Any]:
chart = ChartDAO.find_by_id(annotation_layer["value"])
if not chart:
# pylint: disable=import-outside-toplevel,superfluous-parens
from superset.charts.data.commands.get_data_command import ChartDataCommand

if not (chart := ChartDAO.find_by_id(annotation_layer["value"])):
raise QueryObjectValidationError(_("The chart does not exist"))
if not chart.datasource:
raise QueryObjectValidationError(_("The chart datasource does not exist"))
form_data = chart.form_data.copy()
form_data.update(annotation_layer.get("overrides", {}))

try:
viz_obj = get_viz(
datasource_type=chart.datasource.type,
datasource_id=chart.datasource.id,
form_data=form_data,
force=force,
)
payload = viz_obj.get_payload()
return payload["data"]
if chart.viz_type in viz_types:
Copy link
Member Author

Choose a reason for hiding this comment

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

Same logic was previously but indented.

if not chart.datasource:
raise QueryObjectValidationError(
_("The chart datasource does not exist"),
)

form_data = chart.form_data.copy()
form_data.update(annotation_layer.get("overrides", {}))

payload = get_viz(
datasource_type=chart.datasource.type,
datasource_id=chart.datasource.id,
form_data=form_data,
force=force,
).get_payload()

return payload["data"]

if not (query_context := chart.get_query_context()):
raise QueryObjectValidationError(
_("The chart query context does not exist"),
)

if overrides := annotation_layer.get("overrides"):
if time_grain_sqla := overrides.get("time_grain_sqla"):
for query_object in query_context.queries:
query_object.extras["time_grain_sqla"] = time_grain_sqla

if time_range := overrides.get("time_range"):
from_dttm, to_dttm = get_since_until_from_time_range(time_range)
Copy link
Member Author

Choose a reason for hiding this comment

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

The QueryObject has both time_range and (from_dttm, to_dttm) but I was only able to get the annotation override to adhere to the later.


for query_object in query_context.queries:
query_object.from_dttm = from_dttm
query_object.to_dttm = to_dttm

query_context.force = force
command = ChartDataCommand(query_context)
command.validate()
payload = command.run()
return {"records": payload["queries"][0]["data"]}
except SupersetException as ex:
raise QueryObjectValidationError(error_msg_from_exception(ex)) from ex

Expand Down
10 changes: 10 additions & 0 deletions superset/examples/birth_names.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,16 @@ def create_slices(tbl: SqlaTable) -> tuple[list[Slice], list[Slice]]:
viz_type="table",
metrics=metrics,
),
query_context=get_slice_json(
Copy link
Member Author

Choose a reason for hiding this comment

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

@michael-s-molina even though you were able to run the Celery tests locally without issue, after a few tweaks (increasing timeouts to address the performance degradation on M1 silica) I was able to see the following error:

Screenshot 2023-07-14 at 8 10 46 PM

I don't fully grok where/when the query context is persisted (the only place I could see where the query context was generated from the payload/form-data was via the /api/chart/v1//data/ RESTful API GET endpoint), but it does seem the existence of query context seemed somewhat sporadic. Maybe @villebro can provide more insights in terms of how/where this is generated/persisted as in the example data currently only the "Pivot Table v2" chart (per the slices.query_context column) has the query context explicitly defined yet there are swaths of non-legacy charts.

The TL;DR is similar to the "Pivot Table v2" chart when I explicitly added the query context for the "Daily Totals" chart (which backs the annotation layer in the Cypress test which was failing) the test passed.

default_query_context,
queries=[
{
"columns": ["ds"],
"metrics": metrics,
"time_range": "1983 : 2023",
}
],
),
),
Slice(
**slice_kwargs,
Expand Down
213 changes: 0 additions & 213 deletions superset/viz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,8 @@
get_column_name,
get_column_names,
get_column_names_from_columns,
get_metric_names,
JS_MAX_INTEGER,
merge_extra_filters,
QueryMode,
simple_filter_to_adhoc,
)
from superset.utils.date_parser import get_since_until, parse_past_timedelta
Expand Down Expand Up @@ -701,158 +699,6 @@ def raise_for_access(self) -> None:
security_manager.raise_for_access(viz=self)


class TableViz(BaseViz):

"""A basic html table that is sortable and searchable"""

viz_type = "table"
verbose_name = _("Table View")
credits = 'a <a href="https://github.com/airbnb/superset">Superset</a> original'
is_timeseries = False
enforce_numerical_metrics = False

@deprecated(deprecated_in="3.0")
def process_metrics(self) -> None:
"""Process form data and store parsed column configs.
1. Determine query mode based on form_data params.
- Use `query_mode` if it has a valid value
- Set as RAW mode if `all_columns` is set
- Otherwise defaults to AGG mode
2. Determine output columns based on query mode.
"""
# Verify form data first: if not specifying query mode, then cannot have both
# GROUP BY and RAW COLUMNS.
if (
not self.form_data.get("query_mode")
and self.form_data.get("all_columns")
and (
self.form_data.get("groupby")
or self.form_data.get("metrics")
or self.form_data.get("percent_metrics")
)
):
raise QueryObjectValidationError(
_(
"You cannot use [Columns] in combination with "
"[Group By]/[Metrics]/[Percentage Metrics]. "
"Please choose one or the other."
)
)

super().process_metrics()

self.query_mode: QueryMode = QueryMode.get(
self.form_data.get("query_mode")
) or (
# infer query mode from the presence of other fields
QueryMode.RAW
if len(self.form_data.get("all_columns") or []) > 0
else QueryMode.AGGREGATE
)

columns: list[str] # output columns sans time and percent_metric column
percent_columns: list[str] = [] # percent columns that needs extra computation

if self.query_mode == QueryMode.RAW:
columns = get_metric_names(self.form_data.get("all_columns"))
else:
columns = get_column_names(self.groupby) + get_metric_names(
self.form_data.get("metrics")
)
percent_columns = get_metric_names(
self.form_data.get("percent_metrics") or []
)

self.columns = columns
self.percent_columns = percent_columns
self.is_timeseries = self.should_be_timeseries()

@deprecated(deprecated_in="3.0")
def should_be_timeseries(self) -> bool:
# TODO handle datasource-type-specific code in datasource
conditions_met = self.form_data.get("granularity_sqla") and self.form_data.get(
"time_grain_sqla"
)
if self.form_data.get("include_time") and not conditions_met:
raise QueryObjectValidationError(
_("Pick a granularity in the Time section or " "uncheck 'Include Time'")
)
return bool(self.form_data.get("include_time"))

@deprecated(deprecated_in="3.0")
def query_obj(self) -> QueryObjectDict:
query_obj = super().query_obj()
if self.query_mode == QueryMode.RAW:
query_obj["columns"] = self.form_data.get("all_columns")
order_by_cols = self.form_data.get("order_by_cols") or []
query_obj["orderby"] = [json.loads(t) for t in order_by_cols]
# must disable groupby and metrics in raw mode
query_obj["groupby"] = []
query_obj["metrics"] = []
# raw mode does not support timeseries queries
query_obj["timeseries_limit_metric"] = None
query_obj["timeseries_limit"] = None
query_obj["is_timeseries"] = None
else:
sort_by = self.form_data.get("timeseries_limit_metric")
if sort_by:
sort_by_label = utils.get_metric_name(sort_by)
if sort_by_label not in utils.get_metric_names(query_obj["metrics"]):
query_obj["metrics"].append(sort_by)
query_obj["orderby"] = [
(sort_by, not self.form_data.get("order_desc", True))
]
elif query_obj["metrics"]:
# Legacy behavior of sorting by first metric by default
first_metric = query_obj["metrics"][0]
query_obj["orderby"] = [
(first_metric, not self.form_data.get("order_desc", True))
]
return query_obj

@deprecated(deprecated_in="3.0")
def get_data(self, df: pd.DataFrame) -> VizData:
"""
Transform the query result to the table representation.

:param df: The interim dataframe
:returns: The table visualization data

The interim dataframe comprises of the group-by and non-group-by columns and
the union of the metrics representing the non-percent and percent metrics. Note
the percent metrics have yet to be transformed.
"""
# Transform the data frame to adhere to the UI ordering of the columns and
# metrics whilst simultaneously computing the percentages (via normalization)
# for the percent metrics.
if df.empty:
return None

columns, percent_columns = self.columns, self.percent_columns
if DTTM_ALIAS in df and self.is_timeseries:
columns = [DTTM_ALIAS] + columns
df = pd.concat(
[
df[columns],
(df[percent_columns].div(df[percent_columns].sum()).add_prefix("%")),
],
axis=1,
)
return self.handle_js_int_overflow(
dict(records=df.to_dict(orient="records"), columns=list(df.columns))
)

@staticmethod
@deprecated(deprecated_in="3.0")
def json_dumps(query_obj: Any, sort_keys: bool = False) -> str:
return json.dumps(
query_obj,
default=utils.json_iso_dttm_ser,
sort_keys=sort_keys,
ignore_nan=True,
)


class TimeTableViz(BaseViz):

"""A data table with rich time-series related columns"""
Expand Down Expand Up @@ -1076,65 +922,6 @@ def get_data(self, df: pd.DataFrame) -> VizData:
}


class BigNumberViz(BaseViz):

"""Put emphasis on a single metric with this big number viz"""

viz_type = "big_number"
verbose_name = _("Big Number with Trendline")
credits = 'a <a href="https://github.com/airbnb/superset">Superset</a> original'
is_timeseries = True

@deprecated(deprecated_in="3.0")
def query_obj(self) -> QueryObjectDict:
query_obj = super().query_obj()
metric = self.form_data.get("metric")
if not metric:
raise QueryObjectValidationError(_("Pick a metric!"))
query_obj["metrics"] = [self.form_data.get("metric")]
self.form_data["metric"] = metric
return query_obj

@deprecated(deprecated_in="3.0")
def get_data(self, df: pd.DataFrame) -> VizData:
if df.empty:
return None

df = df.pivot_table(
index=DTTM_ALIAS,
columns=[],
values=self.metric_labels,
dropna=False,
aggfunc=np.min, # looking for any (only) value, preserving `None`
)
df = self.apply_rolling(df)
df[DTTM_ALIAS] = df.index
return super().get_data(df)


class BigNumberTotalViz(BaseViz):

"""Put emphasis on a single metric with this big number viz"""

viz_type = "big_number_total"
verbose_name = _("Big Number")
credits = 'a <a href="https://github.com/airbnb/superset">Superset</a> original'
is_timeseries = False

@deprecated(deprecated_in="3.0")
def query_obj(self) -> QueryObjectDict:
query_obj = super().query_obj()
metric = self.form_data.get("metric")
if not metric:
raise QueryObjectValidationError(_("Pick a metric!"))
query_obj["metrics"] = [self.form_data.get("metric")]
self.form_data["metric"] = metric

# Limiting rows is not required as only one cell is returned
query_obj["row_limit"] = None
return query_obj


class NVD3TimeSeriesViz(NVD3Viz):

"""A rich line chart component with tons of options"""
Expand Down
4 changes: 2 additions & 2 deletions tests/integration_tests/cache_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def test_no_data_cache(self):
app.config["DATA_CACHE_CONFIG"] = {"CACHE_TYPE": "NullCache"}
cache_manager.init_app(app)

slc = self.get_slice("Girls", db.session)
slc = self.get_slice("Top 10 Girl Name Share", db.session)
json_endpoint = "/superset/explore_json/{}/{}/".format(
slc.datasource_type, slc.datasource_id
)
Expand All @@ -73,7 +73,7 @@ def test_slice_data_cache(self):
}
cache_manager.init_app(app)

slc = self.get_slice("Boys", db.session)
slc = self.get_slice("Top 10 Girl Name Share", db.session)
json_endpoint = "/superset/explore_json/{}/{}/".format(
slc.datasource_type, slc.datasource_id
)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests/charts/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1715,7 +1715,7 @@ def test_gets_owned_created_favorited_by_me_filter(self):
)
def test_warm_up_cache(self):
self.login()
slc = self.get_slice("Girls", db.session)
slc = self.get_slice("Top 10 Girl Name Share", db.session)
rv = self.client.put("/api/v1/chart/warm_up_cache", json={"chart_id": slc.id})
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests/charts/commands_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ def test_warm_up_cache_command_chart_not_found(self):

@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_warm_up_cache(self):
slc = self.get_slice("Girls", db.session)
slc = self.get_slice("Top 10 Girl Name Share", db.session)
result = ChartWarmUpCacheCommand(slc.id, None, None).run()
self.assertEqual(
result, {"chart_id": slc.id, "viz_error": None, "viz_status": "success"}
Expand Down
Loading