eladkal commented on code in PR #27197:
URL: https://github.com/apache/airflow/pull/27197#discussion_r1002646168
##########
airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py:
##########
@@ -287,6 +275,12 @@ def __init__(
self.pod_request_obj: k8s.V1Pod | None = None
self.pod: k8s.V1Pod | None = None
+ if "resources" in kwargs:
Review Comment:
BaseOperator resources is used to customized cpu/memory for a task.
Think about a case where you execute PythonOperator but you want to limit it
to specific memory limit of the worker.
In KPO there is no effect (to my understanding) of using BaseOperator
resources since the resource allocation is against the Kubernetes cluster.
BaseOperator resources is dict which is creating a
[Resource](https://github.com/apache/airflow/blob/fd859d138ce9558e19d73cd0cbbe9ef3d7993b64/airflow/utils/operator_resources.py#L31)
object
To answer your question:
`if isinstance(resources, k8s.V1ResourceRequirements):`
Is not case i'm worried about. If user will provide `V1ResourceRequirements`
for `resources` - his code will not work since `resources` expecting dict
https://github.com/apache/airflow/blob/fd859d138ce9558e19d73cd0cbbe9ef3d7993b64/airflow/models/baseoperator.py#L733
The issue I'm trying to protect against is when user is passing K8s resource
allocation as dict via the `resource` parameter. This is something the user
could have done until now because we backport and convert the dict to
V1ResourceRequirements but from now on it will not work.
The issue is that if user didn't notice the change log and continue to pass
resources with K8s parameters like in
https://stackoverflow.com/questions/73959192/airflow-kubernetespodoperator-broken-dag-unexpected-keyword-argument-reque
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
