kalium99 commented on issue #5747: Jinja template + dataframe access from Separator'esque Viz URL: https://github.com/apache/incubator-superset/issues/5747#issuecomment-419280987 @mistercrunch TL;DR; It's probably more secure the way it's currently implemented although its ease of use is not great so maybe it should be disabled by default for that alone? > should this be a feature of the markdown dashboard-native component? Are you referring specifically to the Dashboardv2 Markdown viz (my patch was actually against the 0.26 branch, which was probably a mistake. I will have to rebase it on the correct branch)? I only looked at Dashboard V2 yesterday. As for the Markdown viz in the original Dashboard I did originally consider this. In effect though, it seems to me that the Jinja template is a data visualisation tool. In the sense that we specify datasources, metrics, columns etc, and then we represent that specific data in a way that hopefully tells some story. The markup viz is just to give some context to other parts of the dashboard. This is also reflected in the control panel; the Jinja viz essentially needs the same control panel elements as other data based viz and would differ from that of the Markup viz. > Should this be more of a client-side templating? Meaning you could see the text right away and loading spinners within the text while the async calls are happening. I think Jinja is probably fine easier and opens the power of pandas... That doesn't sound like a bad idea, but It's not where my skill set lies, so I've gone with what I know :-) And I also think that having the power of Pandas is genuinely useful. > clearly some security concerns here, we should probably make this viz type unavailable by default. I'd have to double check, but IIRC if it's done as I've implemented it then there shouldn't be. The `df` is generated via the Query in the control panel, and subject to all the same access checks that any other viz tool is subject too. One of my concerns is actually that people will file a bunch of bugs against it because they have trouble using it. Here are some gotcha's: * If you have the metric `COUNT(*)` then the dataframe column label is 'count', however if your metric is `COUNT(year)` then it's 'COUNT(year)' * Jinja doesn't support all valid Python syntax. This will fail in Jinja `df.groupby('column1').apply(lambda x: x.std())` * Other things that make it not totally user friendly that I haven't come across. PS I do like the thinking of mixing data sources etc, but I'm not sure about the security implications etc. I think it would be good to consider doing something like that at a later date.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
