@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.
[ Full content available at:
https://github.com/apache/incubator-superset/issues/5747 ]
This message was relayed via gitbox.apache.org for [email protected]
