You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/05/19 13:55:37 UTC

[GitHub] [airflow] Dr-Denzy opened a new pull request #15942: Add KPO pod-template-file jinja template support.

Dr-Denzy opened a new pull request #15942:
URL: https://github.com/apache/airflow/pull/15942

This PR adds jinja template support for KubernetesPodOperator
pod-template-file.

fixes #15892

<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.

Feel free to ping committers for the review!

In case of existing issue, reference it using one of the following:

closes: #ISSUE
related: #ISSUE

How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->

---
**^ Add meaningful description above**

Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] oscarrobertson commented on pull request #15942: Add KPO pod-template-file jinja template support.

Posted by GitBox <gi...@apache.org>.

oscarrobertson commented on pull request #15942:
URL: https://github.com/apache/airflow/pull/15942#issuecomment-875676096


   I agree that this is a breaking change, it does feel like something that could be made clearer in the release notes.
   
   I think the code that uses this param to check the extensions also is just checking `endswith`, it's not parsing to find an extension:  
   https://github.com/apache/airflow/blob/main/airflow/models/baseoperator.py#L1047
   
   In other operators the leading `.` of the extension is included: https://github.com/apache/airflow/blob/main/airflow/operators/sql.py#L107
   
   This means this change is probably overly inclusive of parameters being considered files.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] eladkal commented on pull request #15942: Add KPO pod-template-file jinja template support.

Posted by GitBox <gi...@apache.org>.

eladkal commented on pull request #15942:
URL: https://github.com/apache/airflow/pull/15942#issuecomment-875650569


   @theodickson This PR was released in cncf provider 2.0.0 which is a major release (previous was 1.2.0) there can be breaking changes when upgrading a major version. We also mention it in the docs:
   https://airflow.apache.org/docs/apache-airflow-providers/#community-maintained-providers
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] eladkal edited a comment on pull request #15942: Add KPO pod-template-file jinja template support.

Posted by GitBox <gi...@apache.org>.

eladkal edited a comment on pull request #15942:
URL: https://github.com/apache/airflow/pull/15942#issuecomment-875650569


   @theodickson This PR was released in cncf provider 2.0.0 which is a major release (previous was 1.2.0) there can be breaking changes when upgrading a major version. We also mention it in the docs:
   https://airflow.apache.org/docs/apache-airflow-providers/#community-maintained-providers
   
   Though I don't think this PR is something that can be considered as a breaking change


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] theodickson commented on pull request #15942: Add KPO pod-template-file jinja template support.

Posted by GitBox <gi...@apache.org>.

theodickson commented on pull request #15942:
URL: https://github.com/apache/airflow/pull/15942#issuecomment-875639750


   Just pointing out this was a breaking change - if you previously were using values in templated fields which ended in any of these strings, but were _not_ meant to be references to template files, the scheduler now throws an error when it tries to run the task as it tries to treat the string as a local file and load it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] theodickson commented on pull request #15942: Add KPO pod-template-file jinja template support.

Posted by GitBox <gi...@apache.org>.

theodickson commented on pull request #15942:
URL: https://github.com/apache/airflow/pull/15942#issuecomment-875661060


   Sorry I should have been clearer, I realise it was mentioned in the release notes but as a feature not a breaking change. It's not exactly a huge deal and we found the issue and fixed quickly, but it's definitely a breaking change - code we were running successfully before resulted in an error after upgrading!
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] kaxil merged pull request #15942: Add KPO pod-template-file jinja template support.

Posted by GitBox <gi...@apache.org>.

kaxil merged pull request #15942:
URL: https://github.com/apache/airflow/pull/15942


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org