-
Notifications
You must be signed in to change notification settings - Fork 23
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: Escaping Jinja logical statements from assets #205
Conversation
fix: Escaping Jinja logical statements from assets
fixing structure
Update README.rst
Fixing pylint issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if disable_jinja_templating: | ||
config = load_yaml(path_name) | ||
else: | ||
config = render_yaml(path_name, env) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit:
config = load_yaml(path_name) if disable_jinja_templating else render_yaml(path_name, env)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! changed as suggested
if disable_jinja_templating: | ||
overrides = load_yaml(overrides_path) | ||
else: | ||
overrides = render_yaml(overrides_path, env) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed this one as well.
This PR implements escaping for Jinja logical statements (#188), and also adds two new flags:
--disable-jinja-templating
: This flag was added to thesync native
command, so that the CLI doesn't render any Jinja templating from the assets. This is useful in case the assets contain Jinja and were exported via the UI (and therefore the syntax wasn't escaped).--disable-jinja-escaping
: This flag was added to theexport-assets
command, so that the CLI doesn't escape Jinja syntax from the YAML files. This is useful in case the assets contain Jinja and would be imported through the UI.Also, these two flags can be used in case the escaping/rendering logic is incorrectly escaping/rendering incorrect values (like JSON curly brackets, etc).