fix templating of KubernetesPodOperator env_vars field #38522
Replies: 3 comments
-
@potiuk I ran into this same problem of a pr that you previously reviewed and closed due to a bad implementation. I was just trying to use the Anyway, I haven't officially tried out my proposed solution as I need to setup my dev environment to contribute to this repo, but I think I can solve this problem of the previous closed issue/pr. |
Beta Was this translation helpful? Give feedback.
-
Just try it in PR and add tests. There is no reason/need to open issue for it - it makes an extra cognitive load on maintainers to try to understand what you want to do rather than you showing what you want to do in the PR (and tests covering it). Converting it to a discussion as this is not an issue |
Beta Was this translation helpful? Give feedback.
-
Thanks, I'll do that next time. Sorry for the extra noise. Issue fixed in this pr: #39139 |
Beta Was this translation helpful? Give feedback.
-
Apache Airflow version
Other Airflow 2 version (please specify below)
If "Other Airflow 2 version" selected, which one?
2.6.3
What happened?
I happen to be using an older version of Airflow, but I think it still relevant for newer versions.
Basically, this issue: #25827 and this pr: #25829
The type check happens in the
__init__
function before the jinja template strings (or XComArg) gets rendered.What you think should happen instead?
But, I don't think the pr is how I would have solved it. There is no need to call
ast.literal_eval()
as pointed out in the pr review.More simply moving this check
airflow/airflow/providers/cncf/kubernetes/operators/pod.py
Lines 333 to 336 in 5df7a1e
from the
__init__
function (when the jinja templating strings (or XComArg) do not get rendered) to a function that has access to the Pythoncontext
variable (after the jinja templating string (or XComArg) gets rendered). This could be thepre_execute
function, but I don't think that is a good idea in case someone wants to inherit from this operator and implement thepre_execute
function - they would have to remember to callsuper().pre_execute(context)
. So, I think it could be more simply implemented inairflow/airflow/providers/cncf/kubernetes/operators/pod.py
Line 979 in 5df7a1e
function which is called in the
execute
function.How to reproduce
Here is a sample dag using a dynamic task group that works on the
BashOperator
but not on theKubernetesPodOperator
:Operating System
Debian 11
Versions of Apache Airflow Providers
Deployment
Google Cloud Composer
Deployment details
No response
Anything else?
No response
Are you willing to submit PR?
Code of Conduct
Beta Was this translation helpful? Give feedback.
All reactions