-
Notifications
You must be signed in to change notification settings - Fork 26
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(import/export): Improve Jinja handling logic #237
Conversation
Can you explain why this would fail? I thought on import we'd apply Jinja rendering, so that this:
Would get rendered to this before import:
Is the root problem that we don't apply Jinja rendering to certain fields like |
It's actually a problem on rendering the query_context: '{"queries": [{"custom_form_data":{{{ '}}' }}]}' The import throws this error:
The I'm open to testing other approaches if you have any ideas! |
Ah, the problem is that this:
Should be parsed by Jinja as:
But instead is parsed as:
Which is invalid. So the problem is that when exporting we're replacing every |
Let's merge this and then we can improve the export later. |
Since the CLI supports Jinja templating to dynamically modify the asset contents during the import, it automatically escapes existing Jinja templating syntax from assets during the export, so that they are ignored by the CLI renderer until import (and properly preserved to be handled by Superset).
Recently the
query_context
was introduced to chart exports, which is a JSON structure converted to a string. The CLI was incorrectly escaping the JSON structure ({"key_one: {"key_two: "value" {{ '}}' }}
) as if it was Jinja templating syntax during the export, causing the import to fail.This PR improves the Jinja escaping logic, trying to load any string field as a JSON (the most common field is the
query_context
but we have other JSON fields that are exported as strings) so that only Jinja syntax is escaped.It also improves the Jinja rendering logic during import, by performing a YAML -> JSON conversion in case a
TemplateSyntaxError
is raised.