Skip to content
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

Merged
merged 10 commits into from
May 22, 2023
Merged

fix: Escaping Jinja logical statements from assets #205

merged 10 commits into from
May 22, 2023

Conversation

Vitor-Avila
Copy link
Contributor

This PR implements escaping for Jinja logical statements (#188), and also adds two new flags:

  • --disable-jinja-templating: This flag was added to the sync 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 the export-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).

Copy link
Member

@betodealmeida betodealmeida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sagan

Comment on lines 191 to 194
if disable_jinja_templating:
config = load_yaml(path_name)
else:
config = render_yaml(path_name, env)
Copy link
Member

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! changed as suggested

Comment on lines 198 to 201
if disable_jinja_templating:
overrides = load_yaml(overrides_path)
else:
overrides = render_yaml(overrides_path, env)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Copy link
Contributor Author

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.

@Vitor-Avila Vitor-Avila merged commit 2f1e14b into preset-io:main May 22, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants