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 2020/10/23 15:20:00 UTC

[GitHub] [airflow] dimberman opened a new pull request #11784: Add pod_template_override to executor_config

dimberman opened a new pull request #11784:
URL: https://github.com/apache/airflow/pull/11784


   Users will be able to override the base pod_template_file on a per-task
   basis.
   
   <!--
   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] kaxil commented on pull request #11784: Add ability to specify pod_template_file in executor_config

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#issuecomment-722654833


   Cool, can you address the remaining 2 comments please, once you do that, I am happy to approve this PR


----------------------------------------------------------------
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] github-actions[bot] commented on pull request #11784: Add pod_template_override to executor_config

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#issuecomment-722057110


   [The Workflow run](https://github.com/apache/airflow/actions/runs/346593039) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
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] dimberman merged pull request #11784: Add ability to specify pod_template_file in executor_config

Posted by GitBox <gi...@apache.org>.
dimberman merged pull request #11784:
URL: https://github.com/apache/airflow/pull/11784


   


----------------------------------------------------------------
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] github-actions[bot] commented on pull request #11784: Add pod_template_override to executor_config

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#issuecomment-721881251


   [The Workflow run](https://github.com/apache/airflow/actions/runs/346035603) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
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] github-actions[bot] commented on pull request #11784: Add ability to specify pod_template_file in executor_config

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#issuecomment-722666684


   The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run 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.

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



[GitHub] [airflow] github-actions[bot] commented on pull request #11784: Add pod_template_override to executor_config

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#issuecomment-721453771


   [The Workflow run](https://github.com/apache/airflow/actions/runs/344642280) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
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] kaxil commented on pull request #11784: Add pod_template_override to executor_config

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#issuecomment-721444012


   I have rebased your PR on latest Master since we have applied [Black](https://github.com/apache/airflow/commit/4e8f9cc8d02b29c325b8a5a76b4837671bdf5f68) and [PyUpgrade](https://github.com/apache/airflow/commit/8c42cf1b00c90f0d7f11b8a3a455381de8e003c5) on Master.
   


----------------------------------------------------------------
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] dimberman commented on pull request #11784: Add ability to specify pod_template_file in executor_config

Posted by GitBox <gi...@apache.org>.
dimberman commented on pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#issuecomment-722653187


   @kaxil changed


----------------------------------------------------------------
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] kaxil commented on a change in pull request #11784: Add pod_template_override to executor_config

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#discussion_r518274604



##########
File path: airflow/executors/kubernetes_executor.py
##########
@@ -505,6 +506,20 @@ def create_pod_id(dag_id: str, task_id: str) -> str:
     return safe_dag_id + safe_task_id
 
 
+def get_base_pod_from_template(pod_template_file: Optional[str], kube_config: Any) -> k8s.V1Pod:
+    """
+    Reads either the pod_template_file set in the executor_config or the base pod_template_file
+    set in the airflow.cfg to craft a "base pod" that will be used by the KubernetesExecutor
+    @param pod_template_file: absolute path to a pod_template_file.yaml or None
+    @param kube_config:
+    @return:

Review comment:
       we use `:param` format




----------------------------------------------------------------
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] kaxil commented on a change in pull request #11784: Add pod_template_override to executor_config

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#discussion_r518373028



##########
File path: airflow/executors/kubernetes_executor.py
##########
@@ -505,6 +506,21 @@ def create_pod_id(dag_id: str, task_id: str) -> str:
     return safe_dag_id + safe_task_id
 
 
+def get_base_pod_from_template(pod_template_file: Optional[str], kube_config: Any) -> k8s.V1Pod:
+    """
+    Reads either the pod_template_file set in the executor_config or the base pod_template_file
+    set in the airflow.cfg to craft a "base pod" that will be used by the KubernetesExecutor
+
+    :param pod_template_file: absolute path to a pod_template_file.yaml or None
+    :param kube_config:
+    :return:

Review comment:
       same here




----------------------------------------------------------------
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] github-actions[bot] commented on pull request #11784: Add pod_template_override to executor_config

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#issuecomment-722644241


   [The Workflow run](https://github.com/apache/airflow/actions/runs/348279252) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
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] dimberman commented on a change in pull request #11784: Add pod_template_override to executor_config

Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#discussion_r518340032



##########
File path: airflow/executors/kubernetes_executor.py
##########
@@ -505,6 +506,20 @@ def create_pod_id(dag_id: str, task_id: str) -> str:
     return safe_dag_id + safe_task_id
 
 
+def get_base_pod_from_template(pod_template_file: Optional[str], kube_config: Any) -> k8s.V1Pod:
+    """
+    Reads either the pod_template_file set in the executor_config or the base pod_template_file
+    set in the airflow.cfg to craft a "base pod" that will be used by the KubernetesExecutor
+    @param pod_template_file: absolute path to a pod_template_file.yaml or None
+    @param kube_config:
+    @return:

Review comment:
       @kaxil fixed




----------------------------------------------------------------
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] dimberman removed a comment on pull request #11784: Add ability to specify pod_template_file in executor_config

Posted by GitBox <gi...@apache.org>.
dimberman removed a comment on pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#issuecomment-722660108


   @dimberman adressed coments


----------------------------------------------------------------
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] dimberman commented on pull request #11784: Add ability to specify pod_template_file in executor_config

Posted by GitBox <gi...@apache.org>.
dimberman commented on pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#issuecomment-722660108






----------------------------------------------------------------
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] kaxil commented on a change in pull request #11784: Add pod_template_override to executor_config

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11784:
URL: https://github.com/apache/airflow/pull/11784#discussion_r518372830



##########
File path: airflow/executors/kubernetes_executor.py
##########
@@ -505,6 +506,21 @@ def create_pod_id(dag_id: str, task_id: str) -> str:
     return safe_dag_id + safe_task_id
 
 
+def get_base_pod_from_template(pod_template_file: Optional[str], kube_config: Any) -> k8s.V1Pod:
+    """
+    Reads either the pod_template_file set in the executor_config or the base pod_template_file
+    set in the airflow.cfg to craft a "base pod" that will be used by the KubernetesExecutor
+
+    :param pod_template_file: absolute path to a pod_template_file.yaml or None
+    :param kube_config:

Review comment:
       Missing description




----------------------------------------------------------------
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