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/24 07:49:28 UTC
[GitHub] [airflow] dimberman opened a new pull request #11815: Render k8s yaml
dimberman opened a new pull request #11815:
URL: https://github.com/apache/airflow/pull/11815
<!--
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 a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519850066
##########
File path: airflow/models/taskinstance.py
##########
@@ -1655,6 +1664,18 @@ def get_rendered_template_fields(self):
"rendering of template_fields."
) from e
+ def get_rendered_k8s_spec(self):
Review comment:
Added in 8d6ef136a
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519946883
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,55 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ if not settings.IS_K8S_OR_K8SCELERY_EXECUTOR:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.get_rendered_k8s_spec()
+ except AirflowException as e: # pylint: disable=broad-except
Review comment:
fixed: https://github.com/apache/airflow/pull/11815/commits/7550315998869849c7c7e32239f722025bce7d37
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,55 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ if not settings.IS_K8S_OR_K8SCELERY_EXECUTOR:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
Review comment:
fixed in https://github.com/apache/airflow/pull/11815/commits/7550315998869849c7c7e32239f722025bce7d37
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.get_rendered_k8s_spec()
+ except AirflowException as e: # pylint: disable=broad-except
+ msg = "Error rendering template: " + escape(e)
+ if e.__cause__: # pylint: disable=using-constant-test
+ msg += Markup("<br><br>OriginalError: ") + escape(e.__cause__)
+ flash(msg, "error")
+ except Exception as e: # pylint: disable=broad-except
+ flash("Error rendering template: " + str(e), "error")
+ title = "Rendered K8s Pod Spec"
+ html_dict = {}
+ renderers = wwwutils.get_attr_renderer()
+ content = yaml.dump(pod_spec)
Review comment:
fixed in https://github.com/apache/airflow/pull/11815/commits/7550315998869849c7c7e32239f722025bce7d37
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518400653
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
Review comment:
@ryanahamilton do we need to utilize this somehow?
@ashb we could also just merge this for the beta and figure that out later. It's not a breaking issue and it's a functionality we want.
----------------------------------------------------------------
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] ryanahamilton commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517644426
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -132,16 +132,21 @@ <h4 class="modal-title" id="taskInstanceModalLabel">
<hr>
</div>
<a id="btn_task" class="btn btn-sm" data-base-url="{{ url_for('Airflow.task') }}">
- Task Instance Details
+ Instance Details
</a>
- <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered') }}">
+ <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered_templates') }}">
Rendered
</a>
- <a id="btn_ti" class="btn btn-sm" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
- Task Instances
- </a>
+ {% if conf_core_executor == 'KubernetesExecutor' %}
Review comment:
I've added back the missing changes. See `airflow.utils.helpers.k8s_or_k8scelery_executor` for how this was handled.
----------------------------------------------------------------
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] mik-laj commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518038216
##########
File path: airflow/models/taskinstance.py
##########
@@ -70,6 +70,15 @@
from airflow.utils.state import State
from airflow.utils.timeout import timeout
+try:
+ from kubernetes.client.api_client import ApiClient
+
+ from airflow.executors.kubernetes_executor import KubeConfig, create_pod_id
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.settings import pod_mutation_hook
Review comment:
No. It alway, bu we should use`from airflow import settings` to avoid cache.
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519825714
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
Review comment:
This returns `dict` right? not str?
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-724071832
[The Workflow run](https://github.com/apache/airflow/actions/runs/354157212) 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 a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517599341
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -132,16 +132,21 @@ <h4 class="modal-title" id="taskInstanceModalLabel">
<hr>
</div>
<a id="btn_task" class="btn btn-sm" data-base-url="{{ url_for('Airflow.task') }}">
- Task Instance Details
+ Instance Details
</a>
- <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered') }}">
+ <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered_templates') }}">
Rendered
</a>
- <a id="btn_ti" class="btn btn-sm" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
- Task Instances
- </a>
+ {% if conf_core_executor == 'KubernetesExecutor' %}
Review comment:
{% if conf_core_executor in ['KubernetesExecutor', 'CeleryKubernetesExexcutor'] %}
might work
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -132,16 +132,21 @@ <h4 class="modal-title" id="taskInstanceModalLabel">
<hr>
</div>
<a id="btn_task" class="btn btn-sm" data-base-url="{{ url_for('Airflow.task') }}">
- Task Instance Details
+ Instance Details
</a>
- <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered') }}">
+ <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered_templates') }}">
Rendered
</a>
- <a id="btn_ti" class="btn btn-sm" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
- Task Instances
- </a>
+ {% if conf_core_executor == 'KubernetesExecutor' %}
Review comment:
https://jinja.palletsprojects.com/en/2.11.x/templates/
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-715876436
cc: @ryanahamilton I still have a small bug on getting the button on the DAG view to work (also would love to merge this with those assets you built :) )
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518284336
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,11 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ if conf_executor in ('KubernetesExecutor', 'CeleryKubernetesExecutor'):
Review comment:
@turbaszek I had to undo this as it was creating a cyclical import
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518393978
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
Review comment:
> if someone goes to a historical DAG run they should see what it looked like at the time of running.
I agree with this
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519395361
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +204,12 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {
+ executor_constants.KUBERNETES_EXECUTOR,
+ executor_constants.CELERY_KUBERNETES_EXECUTOR,
+ }
Review comment:
an `Is_K8S_OR_CELERY` feels like it would fit well in `airflow.settings` -- but doing that without an import cycle might be tricky.
----------------------------------------------------------------
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] ryanahamilton commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517601393
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -132,16 +132,21 @@ <h4 class="modal-title" id="taskInstanceModalLabel">
<hr>
</div>
<a id="btn_task" class="btn btn-sm" data-base-url="{{ url_for('Airflow.task') }}">
- Task Instance Details
+ Instance Details
</a>
- <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered') }}">
+ <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered_templates') }}">
Rendered
</a>
- <a id="btn_ti" class="btn btn-sm" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
- Task Instances
- </a>
+ {% if conf_core_executor == 'KubernetesExecutor' %}
Review comment:
I wrote a helper that handled this—this appears to be an old version of the code. Was possibly lost in the squash. I'll see about salvaging it—might have to rewrite. 😞
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519896492
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
Review comment:
Yes, your tests show this returning an `optional[Dict]`
```
self.assertEqual(expected_pod_yaml, RTIF.get_k8s_pod_yaml(ti=ti))
```
(`expected_pod_yaml` is a python dict.)
----------------------------------------------------------------
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] potiuk commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518922306
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,9 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {'KubernetesExecutor', 'CeleryKubernetesExecutor'}
Review comment:
Yeah. This chain of dependencies is exactly the problem. Constants out of the loader is a good direction 👍
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519351177
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,9 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {'KubernetesExecutor', 'CeleryKubernetesExecutor'}
Review comment:
Also imports inside a function (almost) never create an import cycle.
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-723264680
[The Workflow run](https://github.com/apache/airflow/actions/runs/350103546) 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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517983985
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +85,31 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
+ """
+ Get templated field for a TaskInstance from the RenderedTaskInstanceFields
+ table.
+
+ :param ti: Task Instance
+ :param session: SqlAlchemy Session
+ :return: Rendered Templated TI field
+ """
+ result = (
+ session.query(cls.k8s_pod_yaml)
+ .filter(
+ cls.dag_id == ti.dag_id, cls.task_id == ti.task_id, cls.execution_date == ti.execution_date
+ )
+ .one_or_none()
+ )
+
+ if result:
+ k8s_pod_yaml = result.k8s_pod_yaml
+ return k8s_pod_yaml
+ else:
+ return None
Review comment:
```suggestion
return result.k8s_pod_yaml if result else None
```
How about this oneliner?
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518384680
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
Review comment:
@ashb we store it because the values are different between task runs, so if someone goes to a historical DAG run they should see what it looked like at the time of running.
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519827540
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
+ """
+ Get templated field for a TaskInstance from the RenderedTaskInstanceFields
+ table.
+
+ :param ti: Task Instance
+ :param session: SqlAlchemy Session
+ :return: Rendered Templated TI field
Review comment:
Updated in 4ed8dc611
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
+ """
+ Get templated field for a TaskInstance from the RenderedTaskInstanceFields
Review comment:
Updated in 4ed8dc611
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
Review comment:
Added test in 4ed8dc611
----------------------------------------------------------------
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] ryanahamilton commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r511936895
##########
File path: airflow/www/views.py
##########
@@ -808,11 +808,11 @@ def dag_details(self, session=None):
tags=tags
)
- @expose('/rendered')
+ @expose('/rendered_templates')
Review comment:
1. If we're going to change the URL of this view, I'd suggest we move to hyphens instead of underscores while we're at it. Hyphens are more of a standard pattern these days.
2. Did you make this plural on purpose? The existing template navigation/heading is singular.
3. Do we have to worry about aliasing/redirecting the old route to the new?
```suggestion
@expose('/rendered-templates')
```
##########
File path: airflow/www/views.py
##########
@@ -860,6 +860,53 @@ def rendered(self):
root=root,
title=title)
+ @expose('/rendered_k8s')
Review comment:
Use hyphen for this one too:
```suggestion
@expose('/rendered-k8s')
```
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-722505679
[The Workflow run](https://github.com/apache/airflow/actions/runs/347916150) 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 a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518172283
##########
File path: airflow/migrations/versions/45ba3f1493b9_add_k8s_yaml_to_rendered_templates.py
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add-k8s-yaml-to-rendered-templates
+
+Revision ID: 45ba3f1493b9
+Revises: 364159666cbd
+Create Date: 2020-10-23 23:01:52.471442
+
+"""
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Column
+
+from airflow.settings import json
+
+# revision identifiers, used by Alembic.
+revision = '45ba3f1493b9'
+down_revision = '2c6edca13270'
+branch_labels = None
+depends_on = None
+
+__tablename__ = "rendered_task_instance_fields"
+k8s_pod_yaml = Column('k8s_pod_yaml', sqlalchemy_jsonfield.JSONField(json=json), nullable=True)
+
+
+def upgrade():
+ """Apply add-k8s-yaml-to-rendered-templates"""
+ with op.batch_alter_table(__tablename__, schema=None) as batch_op:
+ batch_op.add_column(k8s_pod_yaml)
Review comment:
https://github.com/apache/airflow/blob/31dc6cf82734690bf95ade4554e7ebb183055311/airflow/migrations/versions/004c1210f153_increase_queue_name_size_limit.py#L42-L44
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-724142132
> @kaxil - you might want to rebase and force-push again - the #12205 merged should help with the backport packages failing.
Done, thanks
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518422998
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,9 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {'KubernetesExecutor', 'CeleryKubernetesExecutor'}
Review comment:
This was not addressed. We should use `ExecutorLoader.KubernetesExecutor` instead of hardcoding the name. What is more I would be in favour of two things:
a) move this to `executor_loader.py` module
b) Create constant KUBERNETES_EXECUTORS = {...}
Then we can do
```py
def is_k8s_or_k8scelery_executor() -> bool:
return conf.get('core', 'EXECUTOR') in KUBERNETES_EXECUTORS
```
No strong opinion if this should be a separate function or `ExecutorsLoader` method.
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519799044
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
+ """
+ Get templated field for a TaskInstance from the RenderedTaskInstanceFields
Review comment:
docstrings needs updating
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-717368121
[The Workflow run](https://github.com/apache/airflow/actions/runs/331437621) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518156002
##########
File path: airflow/models/taskinstance.py
##########
@@ -70,6 +70,15 @@
from airflow.utils.state import State
from airflow.utils.timeout import timeout
+try:
+ from kubernetes.client.api_client import ApiClient
+
+ from airflow.executors.kubernetes_executor import KubeConfig, create_pod_id
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.settings import pod_mutation_hook
Review comment:
These ones do because they need the kubernetes library, which is not a requirement for airflow.
----------------------------------------------------------------
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] ryanahamilton commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r511955036
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -134,9 +134,12 @@ <h4 class="modal-title" id="taskInstanceModalLabel">
<a id="btn_task" class="btn btn-sm" data-base-url="{{ url_for('Airflow.task') }}">
Task Instance Details
</a>
- <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered') }}">
+ <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered_templates') }}">
Rendered
</a>
+ <a id="btn_rendered_k8s" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered_k8s') }}">
+ Rendered K8s Pod Spec
Review comment:
Can we simplify the navigation button text a bit? They're a bit verbose—maybe a word shorter. "K8s Pod Spec", "Rendered K8s Spec", "Rendered Pod Spec"?
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-724150533
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518393302
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
Review comment:
But where do we read 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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518380531
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
Review comment:
Can we generate this pod spec just off what is stored in a Serialised DAG/Task?
----------------------------------------------------------------
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] potiuk commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518922810
##########
File path: airflow/executors/executor_constants.py
##########
@@ -0,0 +1,24 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+LOCAL_EXECUTOR = "LocalExecutor"
Review comment:
:heart:
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-722398191
[The Workflow run](https://github.com/apache/airflow/actions/runs/347608678) 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 merged pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #11815:
URL: https://github.com/apache/airflow/pull/11815
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-719022924
[The Workflow run](https://github.com/apache/airflow/actions/runs/336652539) 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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519897295
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,55 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ if not settings.IS_K8S_OR_K8SCELERY_EXECUTOR:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.get_rendered_k8s_spec()
+ except AirflowException as e: # pylint: disable=broad-except
Review comment:
This isn't a broad except anymore is 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] dimberman commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518376927
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
Review comment:
@ashb how come? It requires metadata in the task instance to generate the pod.
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519827705
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
Review comment:
Added test in f4b79c71f
##########
File path: airflow/models/taskinstance.py
##########
@@ -1668,6 +1689,26 @@ def render_templates(self, context: Optional[Context] = None) -> None:
self.task.render_template_fields(context)
+ def render_k8s_pod_yaml(self) -> Optional[str]:
Review comment:
Test added in f4b79c71f calls this method and verifies the generated POD JSON
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519913809
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
Review comment:
Yup, left the comment: https://github.com/apache/airflow/pull/11815#discussion_r519825714 but forgot to update
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519773564
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
Review comment:
I will add them in a bit
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-719118200
[The Workflow run](https://github.com/apache/airflow/actions/runs/336944853) 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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518423559
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,9 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {'KubernetesExecutor', 'CeleryKubernetesExecutor'}
Review comment:
@turbaszek any idea how we can do that without creating a circular import? that's what happened when we tried.
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517597168
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -132,16 +132,21 @@ <h4 class="modal-title" id="taskInstanceModalLabel">
<hr>
</div>
<a id="btn_task" class="btn btn-sm" data-base-url="{{ url_for('Airflow.task') }}">
- Task Instance Details
+ Instance Details
</a>
- <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered') }}">
+ <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered_templates') }}">
Rendered
</a>
- <a id="btn_ti" class="btn btn-sm" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
- Task Instances
- </a>
+ {% if conf_core_executor == 'KubernetesExecutor' %}
Review comment:
CeleryKubernetesExexcutor too
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-724142638
Np at all, all good :)
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518239725
##########
File path: airflow/kubernetes/kubernetes_helper_functions.py
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+def _strip_unsafe_kubernetes_special_chars(string: str) -> str:
+ """
+ Kubernetes only supports lowercase alphanumeric characters, "-" and "." in
+ the pod name.
+ However, there are special rules about how "-" and "." can be used so let's
+ only keep
+ alphanumeric chars see here for detail:
+ https://kubernetes.io/docs/concepts/overview/working-with-objects/names/
+
+ :param string: The requested Pod name
+ :return: ``str`` Pod name stripped of any unsafe characters
+ """
+ return ''.join(ch.lower() for ind, ch in enumerate(string) if ch.isalnum())
Review comment:
```suggestion
return ''.join(ch.lower() for ch in list(string) if ch.isalnum())
```
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517598759
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -132,16 +132,21 @@ <h4 class="modal-title" id="taskInstanceModalLabel">
<hr>
</div>
<a id="btn_task" class="btn btn-sm" data-base-url="{{ url_for('Airflow.task') }}">
- Task Instance Details
+ Instance Details
</a>
- <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered') }}">
+ <a id="btn_rendered" class="btn btn-sm" data-base-url="{{ url_for('Airflow.rendered_templates') }}">
Rendered
</a>
- <a id="btn_ti" class="btn btn-sm" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
- Task Instances
- </a>
+ {% if conf_core_executor == 'KubernetesExecutor' %}
Review comment:
@ryanahamilton how do we do or statements 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] kaxil commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519772896
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
Review comment:
There are currently no tests for this method
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517657403
##########
File path: airflow/models/taskinstance.py
##########
@@ -1665,6 +1665,33 @@ def render_templates(self, context: Optional[Context] = None) -> None:
self.task.render_template_fields(context)
+ def render_k8s_pod_yaml(self) -> Optional[str]:
+ """Render k8s pod yaml"""
+ from kubernetes.client.api_client import ApiClient
+
+ from airflow.executors.kubernetes_executor import KubeConfig, create_pod_id
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.settings import pod_mutation_hook
+
+ kube_config = KubeConfig()
+ pod = PodGenerator.construct_pod(
+ dag_id=self.dag_id,
+ task_id=self.task_id,
+ pod_id=create_pod_id(self.dag_id, self.task_id),
+ try_number=self.try_number,
+ kube_image=kube_config.kube_image,
+ date=self.execution_date,
+ command=self.command_as_list(),
+ pod_override_object=PodGenerator.from_obj(self.executor_config),
+ scheduler_job_id="worker-config",
+ namespace=kube_config.executor_namespace,
+ base_worker_pod=PodGenerator.deserialize_model_file(kube_config.pod_template_file),
+ )
+ pod_mutation_hook(pod)
+ api_client = ApiClient()
+ sanitized_pod = api_client.sanitize_for_serialization(pod)
Review comment:
```suggestion
sanitized_pod = ApiClient().sanitize_for_serialization(pod)
```
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519773207
##########
File path: airflow/models/taskinstance.py
##########
@@ -1668,6 +1689,26 @@ def render_templates(self, context: Optional[Context] = None) -> None:
self.task.render_template_fields(context)
+ def render_k8s_pod_yaml(self) -> Optional[str]:
Review comment:
There are currently no tests for this method
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519741276
##########
File path: airflow/migrations/versions/45ba3f1493b9_add_k8s_yaml_to_rendered_templates.py
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add-k8s-yaml-to-rendered-templates
+
+Revision ID: 45ba3f1493b9
+Revises: 364159666cbd
+Create Date: 2020-10-23 23:01:52.471442
+
+"""
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Column
+
+from airflow.settings import json
+
+# revision identifiers, used by Alembic.
+revision = '45ba3f1493b9'
+down_revision = '2c6edca13270'
+branch_labels = None
+depends_on = None
+
+__tablename__ = "rendered_task_instance_fields"
+k8s_pod_yaml = Column('k8s_pod_yaml', sqlalchemy_jsonfield.JSONField(json=json), nullable=True)
+
+
+def upgrade():
+ """Apply add-k8s-yaml-to-rendered-templates"""
+ with op.batch_alter_table(__tablename__, schema=None) as batch_op:
+ batch_op.add_column(k8s_pod_yaml)
Review comment:
Actually, this is fine since you are passing the variable that is Column type which contains the type too on L40
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518827243
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,9 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {'KubernetesExecutor', 'CeleryKubernetesExecutor'}
Review comment:
@potiuk @turbaszek would it be sufficient if I just make ExecutorLoader an import within the function? Though I'm not sure if that would cause runtime issues...
----------------------------------------------------------------
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] potiuk commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519354705
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,9 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {'KubernetesExecutor', 'CeleryKubernetesExecutor'}
Review comment:
> Also imports inside a function (almost) never create an import cycle.
But I am of the opinion that often it is hiding the problem rather than solving it. But that's my opinion, not a fact.
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518380031
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
Review comment:
Much like Rendered TaskInstance fields - once it's run, the db holds exactly was was used (rather than the current rendering)
But either way if we aren't using that col for display: why are we storing it at all?
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518172283
##########
File path: airflow/migrations/versions/45ba3f1493b9_add_k8s_yaml_to_rendered_templates.py
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add-k8s-yaml-to-rendered-templates
+
+Revision ID: 45ba3f1493b9
+Revises: 364159666cbd
+Create Date: 2020-10-23 23:01:52.471442
+
+"""
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Column
+
+from airflow.settings import json
+
+# revision identifiers, used by Alembic.
+revision = '45ba3f1493b9'
+down_revision = '2c6edca13270'
+branch_labels = None
+depends_on = None
+
+__tablename__ = "rendered_task_instance_fields"
+k8s_pod_yaml = Column('k8s_pod_yaml', sqlalchemy_jsonfield.JSONField(json=json), nullable=True)
+
+
+def upgrade():
+ """Apply add-k8s-yaml-to-rendered-templates"""
+ with op.batch_alter_table(__tablename__, schema=None) as batch_op:
+ batch_op.add_column(k8s_pod_yaml)
Review comment:
https://github.com/apache/airflow/blob/master/airflow/migrations/versions/004c1210f153_increase_queue_name_size_limit.py#L42-L44
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r516834068
##########
File path: airflow/www/views.py
##########
@@ -890,6 +890,57 @@ def rendered(self):
root=root,
title=title)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered Dag."""
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
+ except AirflowException as e: # pylint: disable=broad-except
+ msg = "Error rendering template: " + escape(e)
+ if e.__cause__: # pylint: disable=using-constant-test
+ msg += Markup("<br><br>OriginalError: ") + escape(e.__cause__)
+ flash(msg, "error")
+ except Exception as e: # pylint: disable=broad-except
+ flash("Error rendering template: " + str(e), "error")
+ title = "Rendered K8s Pod Spec"
+ html_dict = {}
+ # renderers = wwwutils.get_attr_renderer()
+ # content = json.dumps(pod_spec, sort_keys=True, indent=4)
Review comment:
is this needed?
##########
File path: airflow/www/views.py
##########
@@ -890,6 +890,57 @@ def rendered(self):
root=root,
title=title)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered Dag."""
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
+ except AirflowException as e: # pylint: disable=broad-except
+ msg = "Error rendering template: " + escape(e)
+ if e.__cause__: # pylint: disable=using-constant-test
+ msg += Markup("<br><br>OriginalError: ") + escape(e.__cause__)
+ flash(msg, "error")
+ except Exception as e: # pylint: disable=broad-except
+ flash("Error rendering template: " + str(e), "error")
+ title = "Rendered K8s Pod Spec"
+ html_dict = {}
+ # renderers = wwwutils.get_attr_renderer()
+ # content = json.dumps(pod_spec, sort_keys=True, indent=4)
+ renderers = wwwutils.get_attr_renderer()
+ import yaml
Review comment:
Out of place import
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518423525
##########
File path: airflow/migrations/versions/45ba3f1493b9_add_k8s_yaml_to_rendered_templates.py
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add-k8s-yaml-to-rendered-templates
+
+Revision ID: 45ba3f1493b9
+Revises: 364159666cbd
+Create Date: 2020-10-23 23:01:52.471442
+
+"""
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Column
+
+from airflow.settings import json
+
+# revision identifiers, used by Alembic.
+revision = '45ba3f1493b9'
+down_revision = '2c6edca13270'
+branch_labels = None
+depends_on = None
+
+__tablename__ = "rendered_task_instance_fields"
+k8s_pod_yaml = Column('k8s_pod_yaml', sqlalchemy_jsonfield.JSONField(json=json), nullable=True)
+
+
+def upgrade():
+ """Apply add-k8s-yaml-to-rendered-templates"""
+ with op.batch_alter_table(__tablename__, schema=None) as batch_op:
+ batch_op.add_column(k8s_pod_yaml)
Review comment:
And this one @dimberman I think need attention
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518158845
##########
File path: airflow/migrations/versions/45ba3f1493b9_add_k8s_yaml_to_rendered_templates.py
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add-k8s-yaml-to-rendered-templates
+
+Revision ID: 45ba3f1493b9
+Revises: 364159666cbd
+Create Date: 2020-10-23 23:01:52.471442
+
+"""
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Column
+
+from airflow.settings import json
+
+# revision identifiers, used by Alembic.
+revision = '45ba3f1493b9'
+down_revision = '2c6edca13270'
+branch_labels = None
+depends_on = None
+
+__tablename__ = "rendered_task_instance_fields"
+k8s_pod_yaml = Column('k8s_pod_yaml', sqlalchemy_jsonfield.JSONField(json=json), nullable=True)
+
+
+def upgrade():
+ """Apply add-k8s-yaml-to-rendered-templates"""
+ with op.batch_alter_table(__tablename__, schema=None) as batch_op:
+ batch_op.add_column(k8s_pod_yaml)
Review comment:
@turbaszek what would that look like?
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517984522
##########
File path: airflow/models/taskinstance.py
##########
@@ -70,6 +70,15 @@
from airflow.utils.state import State
from airflow.utils.timeout import timeout
+try:
+ from kubernetes.client.api_client import ApiClient
+
+ from airflow.executors.kubernetes_executor import KubeConfig, create_pod_id
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.settings import pod_mutation_hook
Review comment:
Do those imports have to be in try / except clause?
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518394647
##########
File path: airflow/migrations/versions/45ba3f1493b9_add_k8s_yaml_to_rendered_templates.py
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add-k8s-yaml-to-rendered-templates
+
+Revision ID: 45ba3f1493b9
+Revises: 364159666cbd
+Create Date: 2020-10-23 23:01:52.471442
+
+"""
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Column
+
+from airflow.settings import json
+
+# revision identifiers, used by Alembic.
+revision = '45ba3f1493b9'
+down_revision = '2c6edca13270'
+branch_labels = None
+depends_on = None
+
+__tablename__ = "rendered_task_instance_fields"
+k8s_pod_yaml = Column('k8s_pod_yaml', sqlalchemy_jsonfield.JSONField(json=json), nullable=True)
+
+
+def upgrade():
+ """Apply add-k8s-yaml-to-rendered-templates"""
+ with op.batch_alter_table(__tablename__, schema=None) as batch_op:
+ batch_op.add_column(k8s_pod_yaml)
Review comment:
This still needs to be addressed
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518423525
##########
File path: airflow/migrations/versions/45ba3f1493b9_add_k8s_yaml_to_rendered_templates.py
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add-k8s-yaml-to-rendered-templates
+
+Revision ID: 45ba3f1493b9
+Revises: 364159666cbd
+Create Date: 2020-10-23 23:01:52.471442
+
+"""
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Column
+
+from airflow.settings import json
+
+# revision identifiers, used by Alembic.
+revision = '45ba3f1493b9'
+down_revision = '2c6edca13270'
+branch_labels = None
+depends_on = None
+
+__tablename__ = "rendered_task_instance_fields"
+k8s_pod_yaml = Column('k8s_pod_yaml', sqlalchemy_jsonfield.JSONField(json=json), nullable=True)
+
+
+def upgrade():
+ """Apply add-k8s-yaml-to-rendered-templates"""
+ with op.batch_alter_table(__tablename__, schema=None) as batch_op:
+ batch_op.add_column(k8s_pod_yaml)
Review comment:
And this one I think needs attention @dimberman
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519828221
##########
File path: airflow/models/taskinstance.py
##########
@@ -1668,6 +1689,26 @@ def render_templates(self, context: Optional[Context] = None) -> None:
self.task.render_template_fields(context)
+ def render_k8s_pod_yaml(self) -> Optional[str]:
Review comment:
Test added in 4ed8dc611 calls this method and verifies the generated POD JSON
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517984991
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,11 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ if conf_executor in ('KubernetesExecutor', 'CeleryKubernetesExecutor'):
Review comment:
Let use ExecutorLoader instead of hardcoding names of executors
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517647016
##########
File path: airflow/models/taskinstance.py
##########
@@ -1665,6 +1665,33 @@ def render_templates(self, context: Optional[Context] = None) -> None:
self.task.render_template_fields(context)
+ def render_k8s_pod_yaml(self) -> Optional[str]:
+ """Render k8s pod yaml"""
+ from kubernetes.client.api_client import ApiClient
+
+ from airflow.executors.kubernetes_executor import KubeConfig, create_pod_id
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.settings import pod_mutation_hook
+
+ kube_config = KubeConfig()
+ pod = PodGenerator.construct_pod(
+ dag_id=self.dag_id,
+ task_id=self.task_id,
+ pod_id=create_pod_id(self.dag_id, self.task_id),
+ try_number=self.try_number,
+ kube_image=kube_config.kube_image,
+ date=self.execution_date,
+ command=self.command_as_list(),
+ pod_override_object=PodGenerator.from_obj(self.executor_config),
+ scheduler_job_id="worker-config",
+ namespace=kube_config.executor_namespace,
+ base_worker_pod=PodGenerator.deserialize_model_file(kube_config.pod_template_file),
+ )
+ pod_mutation_hook(pod)
+ api_client = ApiClient()
Review comment:
It probably doesn't matter here as we aren't making an requests with this one.
----------------------------------------------------------------
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] potiuk commented on pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-724141466
@kaxil - you might want to rebase and force-push again - the https://github.com/apache/airflow/pull/12205 merged should help with the backport packages failing.
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-715878101
[The Workflow run](https://github.com/apache/airflow/actions/runs/325640704) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport 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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517984991
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,11 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ if conf_executor in ('KubernetesExecutor', 'CeleryKubernetesExecutor'):
Review comment:
Let use ExecutorLoader instead of hardcoding names of executors na probably we should move this function there?
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-722459372
[The Workflow run](https://github.com/apache/airflow/actions/runs/347780621) 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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-722472993
[The Workflow run](https://github.com/apache/airflow/actions/runs/347819785) 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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517982817
##########
File path: airflow/migrations/versions/45ba3f1493b9_add_k8s_yaml_to_rendered_templates.py
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add-k8s-yaml-to-rendered-templates
+
+Revision ID: 45ba3f1493b9
+Revises: 364159666cbd
+Create Date: 2020-10-23 23:01:52.471442
+
+"""
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Column
+
+from airflow.settings import json
+
+# revision identifiers, used by Alembic.
+revision = '45ba3f1493b9'
+down_revision = '2c6edca13270'
+branch_labels = None
+depends_on = None
+
+__tablename__ = "rendered_task_instance_fields"
+k8s_pod_yaml = Column('k8s_pod_yaml', sqlalchemy_jsonfield.JSONField(json=json), nullable=True)
+
+
+def upgrade():
+ """Apply add-k8s-yaml-to-rendered-templates"""
+ with op.batch_alter_table(__tablename__, schema=None) as batch_op:
+ batch_op.add_column(k8s_pod_yaml)
Review comment:
I think we will need type information for MySQL database
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517600610
##########
File path: airflow/models/taskinstance.py
##########
@@ -1665,6 +1665,33 @@ def render_templates(self, context: Optional[Context] = None) -> None:
self.task.render_template_fields(context)
+ def render_k8s_pod_yaml(self) -> Optional[str]:
+ """Render k8s pod yaml"""
+ from kubernetes.client.api_client import ApiClient
+
+ from airflow.executors.kubernetes_executor import KubeConfig, create_pod_id
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.settings import pod_mutation_hook
+
+ kube_config = KubeConfig()
+ pod = PodGenerator.construct_pod(
+ dag_id=self.dag_id,
+ task_id=self.task_id,
+ pod_id=create_pod_id(self.dag_id, self.task_id),
+ try_number=self.try_number,
+ kube_image=kube_config.kube_image,
+ date=self.execution_date,
+ command=self.command_as_list(),
+ pod_override_object=PodGenerator.from_obj(self.executor_config),
+ scheduler_job_id="worker-config",
+ namespace=kube_config.executor_namespace,
+ base_worker_pod=PodGenerator.deserialize_model_file(kube_config.pod_template_file),
+ )
+ pod_mutation_hook(pod)
+ api_client = ApiClient()
Review comment:
Should we not be using Client from https://github.com/apache/airflow/blob/master/airflow/kubernetes/kube_client.py#L97-L101
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517647977
##########
File path: airflow/models/renderedtifields.py
##########
@@ -49,6 +50,7 @@ def __init__(self, ti: TaskInstance, render_templates=True):
self.ti = ti
if render_templates:
ti.render_templates()
+ self.k8s_pod_yaml = ti.render_k8s_pod_yaml()
Review comment:
This is going to cause an exception when the Kube modules aren't available.
```suggestion
if is_k8s_or_k8scelery_executor():
self.k8s_pod_yaml = ti.render_k8s_pod_yaml()
```
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +83,31 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
+ """
+ Get templated field for a TaskInstance from the RenderedTaskInstanceFields
+ table.
+
+ :param ti: Task Instance
+ :param session: SqlAlchemy Session
+ :return: Rendered Templated TI field
+ """
+ result = (
+ session.query(cls.rendered_fields)
Review comment:
This is going to only select the rendered_fields, not the `k8s_pod_yaml` field.
```suggestion
session.query(cls.k8s_pod_yaml)
```
You probably need to improve on the tests to cover this as this wasn't caught.
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-721443683
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518375530
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
Review comment:
Shouldn't this be using `RenceredTIFIelds.get_k8s_pod_yaml`?
Or if not, we should delete that function.
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519895029
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.get_rendered_k8s_spec()
+ except AirflowException as e: # pylint: disable=broad-except
+ msg = "Error rendering template: " + escape(e)
+ if e.__cause__: # pylint: disable=using-constant-test
+ msg += Markup("<br><br>OriginalError: ") + escape(e.__cause__)
+ flash(msg, "error")
+ except Exception as e: # pylint: disable=broad-except
+ flash("Error rendering template: " + str(e), "error")
+ title = "Rendered K8s Pod Spec"
+ html_dict = {}
+ renderers = wwwutils.get_attr_renderer()
+ content = yaml.dump(pod_spec)
Review comment:
Did you fix this?
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519897005
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,55 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ if not settings.IS_K8S_OR_K8SCELERY_EXECUTOR:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
Review comment:
Do we need to copy this?
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519360415
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +204,12 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {
+ executor_constants.KUBERNETES_EXECUTOR,
+ executor_constants.CELERY_KUBERNETES_EXECUTOR,
+ }
Review comment:
Wouldn't it be better to move it to `executor_constants` as `IS_K8S_OR_CELERY = conf.get('core', 'EXECUTOR') in CELERY_OR_K8S_EXECUTORS`?
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519773095
##########
File path: airflow/models/taskinstance.py
##########
@@ -1655,6 +1664,18 @@ def get_rendered_template_fields(self):
"rendering of template_fields."
) from e
+ def get_rendered_k8s_spec(self):
Review comment:
There are currently no tests for this method
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518394292
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.render_k8s_pod_yaml()
Review comment:
But where do we use `RenderedTaskInstanceFields.get_k8s_pod_yaml` method?
----------------------------------------------------------------
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] potiuk commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518785922
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,9 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {'KubernetesExecutor', 'CeleryKubernetesExecutor'}
Review comment:
Can you tlell what were the circular imports. Usually (not always) circular imports can be resolved via applying single responsibility principle and splitting out or joining certain classes.
Usually, I solve them in the way that I put closely coupled classes together - in a single module, but those classes that are not coupled in separate modules and make sure one those modules do not import each other. Sometimes (and usually it results in a cleaner architecture and better "single responsibility principle" application) it results in having to add some kind of mediator between those classes.
However in this case we have a big architectural problem as the "conf" module (which should be merely an util) pulls in half of the airlfow (because it is not designed well to be "just conf"). I tried several times to untangle it with varying level of success, and I am afraid it might be difficult to solve. It's rather difficult and result of historical way of implementing the conf class. I think we should finally solve it at some point in time, but this is quite an effort as it ripples through half of the code of airflow (which is bad IMHO but we have to leave with it until we fix 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] kaxil commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519799502
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
+ """
+ Get templated field for a TaskInstance from the RenderedTaskInstanceFields
+ table.
+
+ :param ti: Task Instance
+ :param session: SqlAlchemy Session
+ :return: Rendered Templated TI field
Review comment:
Needs updating
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r518921277
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,9 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {'KubernetesExecutor', 'CeleryKubernetesExecutor'}
Review comment:
So the problem is tied to the fact that ExecutorLoader imports BaseExecutor, which imports TaskInstance.... which imports helpers...
I implemented one potential solution which is to move these constants out of the ExecutorLoader (which kinda makes sense since we're not loading an executor).
----------------------------------------------------------------
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] ashb commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519894122
##########
File path: airflow/models/renderedtifields.py
##########
@@ -81,6 +84,26 @@ def get_templated_fields(cls, ti: TaskInstance, session: Session = None) -> Opti
else:
return None
+ @classmethod
+ @provide_session
+ def get_k8s_pod_yaml(cls, ti: TaskInstance, session: Session = None) -> Optional[str]:
Review comment:
Since this field is marked as a JSON type doesn't SQLAlchemy do a `json.loads` for us already?
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519766497
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +204,12 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ return conf_executor in {
+ executor_constants.KUBERNETES_EXECUTOR,
+ executor_constants.CELERY_KUBERNETES_EXECUTOR,
+ }
Review comment:
Updated in https://github.com/apache/airflow/pull/11815/commits/aaa04916eef7437dd2ebde339c9d7efacef72dbc
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519752182
##########
File path: airflow/www/views.py
##########
@@ -924,6 +926,56 @@ def rendered(self):
title=title,
)
+ @expose('/rendered-k8s')
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ]
+ )
+ @action_logging
+ def rendered_k8s(self):
+ """Get rendered k8s yaml."""
+ k8s = is_k8s_or_k8scelery_executor()
+ if not k8s:
+ abort(404)
+ dag_id = request.args.get('dag_id')
+ task_id = request.args.get('task_id')
+ execution_date = request.args.get('execution_date')
+ dttm = timezone.parse(execution_date)
+ form = DateTimeForm(data={'execution_date': dttm})
+ root = request.args.get('root', '')
+ logging.info("Retrieving rendered templates.")
+ dag = current_app.dag_bag.get_dag(dag_id)
+ task = copy.copy(dag.get_task(task_id))
+ ti = models.TaskInstance(task=task, execution_date=dttm)
+ try:
+ pod_spec = ti.get_rendered_k8s_spec()
+ except AirflowException as e: # pylint: disable=broad-except
+ msg = "Error rendering template: " + escape(e)
+ if e.__cause__: # pylint: disable=using-constant-test
+ msg += Markup("<br><br>OriginalError: ") + escape(e.__cause__)
+ flash(msg, "error")
+ except Exception as e: # pylint: disable=broad-except
+ flash("Error rendering template: " + str(e), "error")
+ title = "Rendered K8s Pod Spec"
+ html_dict = {}
+ renderers = wwwutils.get_attr_renderer()
+ content = yaml.dump(pod_spec)
Review comment:
This will error with Variable not defined if L953 raises an error
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517985515
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +203,11 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def is_k8s_or_k8scelery_executor() -> bool:
+ """Determines if the executor utilizes k8s."""
+ conf_executor = conf.get('core', 'EXECUTOR')
+ if conf_executor in ('KubernetesExecutor', 'CeleryKubernetesExecutor'):
+ return True
+ return False
Review comment:
We can simply return:
```
return conf_executor in ('KubernetesExecutor', 'CeleryKubernetesExecutor')
```
This is boolean value
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517600098
##########
File path: airflow/models/taskinstance.py
##########
@@ -1665,6 +1665,33 @@ def render_templates(self, context: Optional[Context] = None) -> None:
self.task.render_template_fields(context)
+ def render_k8s_pod_yaml(self) -> Optional[str]:
+ """Render k8s pod yaml"""
+ from kubernetes.client.api_client import ApiClient
+
+ from airflow.executors.kubernetes_executor import KubeConfig, create_pod_id
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.settings import pod_mutation_hook
Review comment:
any reason this imports are 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] kaxil commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r517600098
##########
File path: airflow/models/taskinstance.py
##########
@@ -1665,6 +1665,33 @@ def render_templates(self, context: Optional[Context] = None) -> None:
self.task.render_template_fields(context)
+ def render_k8s_pod_yaml(self) -> Optional[str]:
+ """Render k8s pod yaml"""
+ from kubernetes.client.api_client import ApiClient
+
+ from airflow.executors.kubernetes_executor import KubeConfig, create_pod_id
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.settings import pod_mutation_hook
Review comment:
any reason these imports are 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] kaxil commented on a change in pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#discussion_r519741276
##########
File path: airflow/migrations/versions/45ba3f1493b9_add_k8s_yaml_to_rendered_templates.py
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add-k8s-yaml-to-rendered-templates
+
+Revision ID: 45ba3f1493b9
+Revises: 364159666cbd
+Create Date: 2020-10-23 23:01:52.471442
+
+"""
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Column
+
+from airflow.settings import json
+
+# revision identifiers, used by Alembic.
+revision = '45ba3f1493b9'
+down_revision = '2c6edca13270'
+branch_labels = None
+depends_on = None
+
+__tablename__ = "rendered_task_instance_fields"
+k8s_pod_yaml = Column('k8s_pod_yaml', sqlalchemy_jsonfield.JSONField(json=json), nullable=True)
+
+
+def upgrade():
+ """Apply add-k8s-yaml-to-rendered-templates"""
+ with op.batch_alter_table(__tablename__, schema=None) as batch_op:
+ batch_op.add_column(k8s_pod_yaml)
Review comment:
Actually, this is fine since you are passing the variable that is Column type which contains the type too on L40
I have tested in on MySQL too:
```
INFO [alembic.runtime.migration] Running upgrade 98271e7606e2 -> 52d53670a240, fix_mssql_exec_date_rendered_task_instance_fields_for_MSSQL
INFO [alembic.runtime.migration] Running upgrade 52d53670a240 -> 849da589634d, Prefix DAG permissions.
[2020-11-09 11:33:21,272] {manager.py:727} WARNING - No user yet created, use flask fab command to do it.
[2020-11-09 11:33:29,248] {migration.py:515} INFO - Running upgrade 849da589634d -> 364159666cbd, Add creating_job_id to DagRun table
[2020-11-09 11:33:29,282] {migration.py:515} INFO - Running upgrade 364159666cbd -> 2c6edca13270, Resource based permissions.
[2020-11-09 11:33:29,340] {manager.py:727} WARNING - No user yet created, use flask fab command to do it.
[2020-11-09 11:33:32,472] {migration.py:515} INFO - Running upgrade 2c6edca13270 -> 45ba3f1493b9, add-k8s-yaml-to-rendered-templates
[2020-11-09 11:33:32,814] {dagbag.py:434} INFO - Filling up the DagBag from /root/airflow/dags
```
----------------------------------------------------------------
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] potiuk commented on pull request #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-724142453
Sorry. Seems some unfetched commits got into our way
----------------------------------------------------------------
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 #11815: Render k8s yaml for tasks via the Airflow UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11815:
URL: https://github.com/apache/airflow/pull/11815#issuecomment-716003988
[The Workflow run](https://github.com/apache/airflow/actions/runs/326187966) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport 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