Consider supporting .jinja2 extensions across the project

apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows

https://airflow.apache.org/

Apache License 2.0

37.34k stars 14.34k forks source link

Consider supporting .jinja2 extensions across the project #8603

Open oripwk opened 4 years ago

oripwk commented 4 years ago

Description

At the moment, various operators support reading templates from given filenames (for example, bash_command of BashOperator). However, all of these operators specify extensions such as .sh, .json etc., so it means users need to write Jinja2 templates inside .sh, .json files. A better way to do it would be supporting the .jinja2 extension.

Use case / motivation

The motivation is threefold:

.jinja2 extension is a widespread convention which is supported by various editors, including Github itself, PyCharm and VSCode.
Putting templates inside .jinja2 can help distinguish between regular files and template files just by looking at the directory contents
Working with tools that recognize .jinja2 templates can help reduce errors while authoring templates.

boring-cyborg[bot] commented 4 years ago

Thanks for opening your first issue here! Be sure to follow the issue template!

mik-laj commented 4 years ago

Hi.

It looks good. I think we can accept jinja2 whenever template ext is not empty. Do you want to work on it? Should I assign you to this ticket?

Best regards, Kamil

oripwk commented 4 years ago

Hi,

Yes, I can work on it. You can assign

potiuk commented 4 years ago

Hey @oripwk. I generally like the idea, but can you also write about this one on the devlist?

I think this is quite a fundamental change int the approach for potentially many operators, and I feel we should have more people commenting on it and maybe coming up with some ideas how to handle it well. I think for example that better approach will be to simply support additional .jinja2 extension when you have extensions defined. For example when you have '.sh' extension in template extensions, you'd look in both *.sh and *.sh.jinja2 but not just *.jinja2. This way we might preserve both - nice formatting support from IDE's and semantic information about the actual type of the file. But this is just my opinion and maybe we can discuss it at the devlist.

Woudl you be so kind to write about the proposal at the devlist please ?

oripwk commented 4 years ago

Sure. I also commented on the PR regarding how to generate documentation for this, since it affects many operators. Do you have any suggestion how to deal with it? https://github.com/apache/airflow/pull/8616#issuecomment-621086618

eladkal commented 3 years ago

@kaxil this issue should be resolved by https://github.com/apache/airflow/pull/14603 correct?

kaxil commented 3 years ago

@kaxil this issue should be resolved by #14603 correct?

No, actually this is a bit different i.e. the OP wants to allow using "file.jinja2" instead of "file.bash" or "file.sql" in rendered template fields.

uranusjr commented 3 months ago

This would be irrelevant if AIP-80 is accepted. Instead of inferring from the extension, you’d need to pass in a FileTemplate class explicitly, which can accept any file name.