feat(dataset API): Add parameter to optionally render Jinja macros in API response #30721
Conversation
superset/datasets/api.py
Outdated
| item: Optional[SqlaTable] = self.datamodel.get( | ||
| pk, | ||
| self._base_filters, | ||
| self.show_select_columns, | ||
| self.show_outer_default_load, | ||
| ) | ||
| if not item: | ||
| return self.response_404() | ||
|
|
||
| response: dict[str, Any] = {} | ||
| args = kwargs.get("rison", {}) | ||
| select_cols = args.get(API_SELECT_COLUMNS_RIS_KEY, []) | ||
| pruned_select_cols = [col for col in select_cols if col in self.show_columns] | ||
| self.set_response_key_mappings( | ||
| response, | ||
| self.get, | ||
| args, | ||
| **{API_SELECT_COLUMNS_RIS_KEY: pruned_select_cols}, | ||
| ) | ||
| if pruned_select_cols: | ||
| show_model_schema = self.model2schemaconverter.convert(pruned_select_cols) | ||
| else: | ||
| show_model_schema = self.show_model_schema | ||
|
|
||
| response["id"] = pk | ||
| response[API_RESULT_RES_KEY] = show_model_schema.dump(item, many=False) |
There was a problem hiding this comment.
This is FAB's default implementation for get_headless - ref.
I decided to not simply do self.get_headless() and then modify the response for two reasons:
get_headlessreturns a response object, so while we could just modify theresponse.jsonit feels a little hacky.- Superset's
get_headlessimplementation includes a call tosend_stats_metrics(ref), which wouldn't include the full duration of the API call as the response would be further modified by this method before concluding.
I also thought about just overwriting FAB's pre_get method which is available in case the response needs to be modified as part of get_headless, but we need the Database associated with this dataset, and if the user chooses to not include the database.id in the response then pre_get wouldn't have this info.
There was a problem hiding this comment.
LGTM, however, it's worth remembering that the rendered query can change, considerably even, if it has lots of conditional Jinja logic. It's pretty common to see additional where clauses added if certain filter parameters are set in the chart data request. Just something to keep in mind.
@villebro that's true. I actually decided to include both the regular |
superset/datasets/api.py
Outdated
| except TemplateSyntaxError as ex: | ||
| raise SupersetTemplateException( | ||
| f"Unable to render expression from dataset {item_type}.", | ||
| ) from ex |
There was a problem hiding this comment.
Could possibly be simplified to:
items = [
("query", "sql", "rendered_sql", processor.process_template),
("metric", "metrics", "metrics", render_item_list),
("column", "columns", "columns", render_item_list),
]
for item_type, key, new_key, func in items:
if key not in data: # or if not data.get(key), potentially
continue
try:
data[new_key] = func(data[key])
except TemplateSyntaxError as ex:
raise SupersetTemplateException(
f"Unable to render expression from dataset {item_type}.",
) from exBut it's perfectly fine as is.
There was a problem hiding this comment.
That's cool! Let me implement this 🙏
SUMMARY
This PR adds support for optionally rendering Jinja macros in the API response when getting a dataset. The motivation for this feature is that external tools might be fetching dataset metadata via the API to define lineage/etc, and in case Jinja is used it's not possible to fully parse the SQL query to identify dependencies/etc. An example query:
The query parameter
include_rendered_sql(bool) can be used to add keys to the response:rendered_sqlfor the rendered virtual dataset SQL query.rendered_expressionfor the rendered expression frommetricsandcolumns.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before
After
TESTING INSTRUCTIONS
Testing added.
ADDITIONAL INFORMATION