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.

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(dashboard): 500 error caused by data_for_slices API #16053

Merged
merged 1 commit into from
Aug 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 4 additions & 5 deletions superset/connectors/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,9 @@ def data_for_slices(self, slices: List[Slice]) -> Dict[str, Any]:
column_names = set()
for slc in slices:
form_data = slc.form_data

# pull out all required metrics from the form_data
for param in METRIC_FORM_DATA_PARAMS:
for metric in utils.get_iterable(form_data.get(param) or []):
for metric_param in METRIC_FORM_DATA_PARAMS:
for metric in utils.get_iterable(form_data.get(metric_param) or []):
metric_names.add(utils.get_metric_name(metric))
if utils.is_adhoc_metric(metric):
column_names.add(
Expand All @@ -308,8 +307,8 @@ def data_for_slices(self, slices: List[Slice]) -> Dict[str, Any]:

column_names.update(
column
for column in utils.get_iterable(form_data.get(param) or [])
for param in COLUMN_FORM_DATA_PARAMS
for column_param in COLUMN_FORM_DATA_PARAMS
for column in utils.get_iterable(form_data.get(column_param) or [])
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 order of the for loop is incorrect. Normally it will throw an undefined variable error but since param was used in other loops above, the code will still run. Just another example of why too many local variables is not a good thing.

)

filtered_metrics = [
Expand Down
12 changes: 3 additions & 9 deletions superset/utils/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1217,10 +1217,10 @@ def get_metric_name(metric: Metric) -> str:


def get_metric_names(metrics: Sequence[Metric]) -> List[str]:
return [get_metric_name(metric) for metric in metrics]
return [metric for metric in map(get_metric_name, metrics) if metric]


def get_main_metric_name(metrics: Sequence[Metric]) -> Optional[str]:
def get_first_metric_name(metrics: Sequence[Metric]) -> Optional[str]:
metric_labels = get_metric_names(metrics)
return metric_labels[0] if metric_labels else None

Expand Down Expand Up @@ -1427,7 +1427,6 @@ def get_iterable(x: Any) -> List[Any]:
:param x: The object
:returns: An iterable representation
"""

return x if isinstance(x, list) else [x]


Expand Down Expand Up @@ -1464,12 +1463,7 @@ def get_column_names_from_metrics(metrics: List[Metric]) -> List[str]:
:param metrics: Ad-hoc metric
:return: column name if simple metric, otherwise None
"""
columns: List[str] = []
for metric in metrics:
column_name = get_column_name_from_metric(metric)
if column_name:
columns.append(column_name)
return columns
return [col for col in map(get_column_name_from_metric, metrics) if col]
Copy link
Member Author

Choose a reason for hiding this comment

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

Bycatch refactor. No real code logic changes.



def extract_dataframe_dtypes(df: pd.DataFrame) -> List[GenericDataType]:
Expand Down
2 changes: 1 addition & 1 deletion superset/viz.py
Original file line number Diff line number Diff line change
Expand Up @@ -1230,7 +1230,7 @@ def query_obj(self) -> QueryObjectDict:
d = super().query_obj()
sort_by = self.form_data.get(
"timeseries_limit_metric"
) or utils.get_main_metric_name(d.get("metrics") or [])
) or utils.get_first_metric_name(d.get("metrics") or [])
is_asc = not self.form_data.get("order_desc")
if sort_by:
sort_by_label = utils.get_metric_name(sort_by)
Expand Down
14 changes: 8 additions & 6 deletions tests/integration_tests/model_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -497,13 +497,15 @@ def test_data_for_slices(self):
slc = (
metadata_db.session.query(Slice)
.filter_by(
datasource_id=tbl.id,
datasource_type=tbl.type,
slice_name="Participants",
datasource_id=tbl.id, datasource_type=tbl.type, slice_name="Genders",
)
.first()
)
data_for_slices = tbl.data_for_slices([slc])
self.assertEqual(len(data_for_slices["columns"]), 0)
self.assertEqual(len(data_for_slices["metrics"]), 1)
self.assertEqual(len(data_for_slices["verbose_map"].keys()), 2)
assert len(data_for_slices["metrics"]) == 1
assert len(data_for_slices["columns"]) == 1
assert data_for_slices["metrics"][0]["metric_name"] == "sum__num"
assert data_for_slices["columns"][0]["column_name"] == "gender"
assert set(data_for_slices["verbose_map"].keys()) == set(
["__timestamp", "sum__num", "gender",]
)