Closed louison closed 3 years ago
Thanks for opening your first issue here! Be sure to follow the issue template!
possibly due to https://github.com/apache/airflow/pull/15942
json was added as templated_ext
which means what Airflow tries to template all fields in template_fields
that contains the suffix json
.
can you see if MyKubernetesPodOperator
as:
class MyKubernetesPodOperator(KubernetesPodOperator):
template_ext = ('.yaml', '.yml', '.json')
solves your problem?
Yup you are right, @eladkal -- would you like to create a PR with that fix :) ?
We should do it now so it is available in next Providers release
I have created https://github.com/apache/airflow/pull/16930 just in case you are busy
I have created #16930 just in case you are busy
Thanks! I wonder if its possible to create a test that will prevent future cases of this problem (test that makes sure any string in temple_ext to any opetator starts with dot). I'm not near my laptop but I think I we have other operators that are missing the dot.
Comment from https://github.com/apache/airflow/issues/17037#issuecomment-881946792 -> seems that there are doubts whether fix in #16930 actually solves the problem. The comment by @raphaelauv suggests that the root cause is not solved and that passing and argument ending with ".json" (including the '.') still causes failure and does not really solve the problem (@louison @kaxil @eladkal ? WDYT?) . I think the change makes it "consistent" with current behaviour of Airflow for all other operators, but maybe indeed there should be a way to disable the "extension" behaviour (but I see it as a new feature, rather than bug).
Yeah this change make it consistent , it's a good MR.
It just do not solve HIS (@louison) problem, and I'm not saying that HIS (@louison) problem should become OURS ( airflow community).
ps: I was just around to give help on checking the RC providers , nothing more
@raphaelauv in your reproduce example:
k = KubernetesPodOperator(
namespace=namespace,
image="hello-world",
labels={"foo": "bar"},
arguments=["vivi.json"],
name="airflow-test-pod",
task_id="task-one",
...
)
Did you actually have a vivi.json
file so that jinja could pickup? The traceback you shared is different then the one posted on this issue. Your traceback says jinja2.exceptions.TemplateNotFound: vivi.json
which means that jinja doesn't find the vivi.json
to process.
Did you actually have a
vivi.json
file so that jinja could pickup? The traceback you shared is different then the one posted on this issue. Your traceback saysjinja2.exceptions.TemplateNotFound: vivi.json
which means that jinja doesn't find thevivi.json
to process.
Yep. That is also my point and I think we agree here @eladkal. I think @louison (can you please confirm that ?) really complained in this issue not that just using pod-ex-minimum_json
and pod-ex-minimum_txt
as the values of the parameter made it fail. IMHO it's reasonable to expect that 'something.json' is a file name and that it will fail if missing (this is how Airflow's 'template_extension' feature works for all other operators too).
Did you actually have a
vivi.json
file so that jinja could pickup? The traceback you shared is different then the one posted on this issue. Your traceback saysjinja2.exceptions.TemplateNotFound: vivi.json
which means that jinja doesn't find thevivi.json
to process.Yep. That is also my point and I think we agree here @eladkal. I think @louison (can you please confirm that ?) really complained in this issue not that just using
pod-ex-minimum_json
andpod-ex-minimum_txt
as the values of the parameter made it fail. IMHO it's reasonable to expect that 'something.json' is a file name and that it will fail if missing (this is how Airflow's 'template_extension' feature works for all other operators too).
That's correct
I honestly do not understand how this behaviour is considered normal. In what case would you expect it to actually load the contents of vivi.json and pass that to the kubernetes pod, instead of just passing the literal string "vivi.json" to it?
Furthermore, how can I pass a string argument to a kubernetes pod operator as an env_var without airflow trying to load it as a file (but only when it ends with json or yaml) ?
See discussion in #17186 - it turned out really to be a bug in the implementation of recursive rendering of parameters in Kubernetes Pod Operator,
You can (and Kubernetes operator actually does) specify specific extensions that are treated differently when processing parameters. This is an individual decision of each operator implementation and it only works for templated fields, so you can still pass '.json` as string for non-templated fields. See https://airflow.apache.org/docs/apache-airflow/stable/howto/custom-operator.html?highlight=template%20fields#templating .
In this case however that was a bug with recursive behaviour for the KPO is wrong and should be improved (but it only affects KPO)
@louison @ericdevries
class MyKubernetesPodOperator(KubernetesPodOperator):
template_fields = tuple(x for x in KubernetesPodOperator.template_fields if x != "arguments")
task = MyKubernetesPodOperator(
# The ID specified for the task.
task_id='pod-ex-minimum_json',
name='pod-ex-minimum_json',
cmds=['echo'],
namespace='default',
arguments=["vivi.json"],
image='gcr.io/gcp-runtimes/ubuntu_18_0_4'
)
Apache Airflow version: 2.1.1
Kubernetes version (if you are using kubernetes) (use
kubectl version
):Environment:
uname -a
): Darwin Louisons-MacBook-Pro.local 20.5.0 Darwin Kernel Version 20.5.0: Sat May 8 05:10:33 PDT 2021; root:xnu-7195.121.3~9/RELEASE_X86_64 x86_64What happened:
While trying to write a simple dag with KubernetesPodExecutor, I noticed that in certain cases, the pod is launched but not always. By investigating a bit more, I found that when the string
".json"
is present in parameters of the KubernetesPodOperator, it will not work. I tried to set up a minimal example to reproduce the bug. I manage to reproduce the bug on my kubernetes cluster and my Airflow instance (if it can help)No error message or log to give here. Here is the logs of the scheduler while trying to execute one run:
Don't hesitate to ask me if you need more info