fix(ingest): handle groupby custom label case #6456
Conversation
github-actions
bot
added
the
ingestion
fix(ingest): handle groupby custom label case
phongvu99
force-pushed
the
superset-fix-ingest-groupbys
branch
from
070f12b to
8de070b
Compare
| @@ -308,6 +308,21 @@ def construct_chart_from_chart_data(self, chart_data): | |||
| group_bys = params.get("groupby", []) or [] | |||
| if isinstance(group_bys, str): | |||
There was a problem hiding this comment.
Can this also be a dictionary?
There was a problem hiding this comment.
So far I've only seen the Superset API returns the groupby as a list. Not sure why there's a check if it's a string in the original code, hence my concern in the previous comment about the fourth case.
| # if the item is a custom label | ||
| if isinstance(item, dict): | ||
| item_value = item.get("label", "") | ||
| if item_value != "": temp_group_bys.append(f"{item_value}_custom_label") |
There was a problem hiding this comment.
Why do we need _custom_label here?
There was a problem hiding this comment.
The _custom_label acts as an identifier since the original label is lost when using a custom label. As mentioned in the description above, a future improvement is to somehow retrieve the lost value, but that's up to the Superset API.
I've replied to the comments above. Thank you for accepting the PR! |
fix(ingest): handle groupby custom label case Co-authored-by: Phong Vu <[email protected]> Co-authored-by: Harshal Sheth <[email protected]>
Summary
This PR fixed the following exception when ingesting from Superset. The problem only happens if the ingested charts' metadata, the groupby parameter is using a custom label. The ChartAPI in Superset returns the groupby param in the following format, causing the exception.
What's changed
I've added a small check to ensure the ingestion won't fail. But the original label is lost, and replaced by a combination of the custom label and "_custom_label" appended at the end as a workaround.
A possible enhancement in the future is to modify the code to find the lost value somehow.
The Exception
Checklist