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.
I think it's for a case where you execute PythonOperator but you want to
limit it to specific memory limit of the worker (But I'm not sure?)
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?
##########
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:
BTW possible BaseOperator resources is never used anyway?
Ash mentioned it in
https://github.com/apache/airflow/issues/16563#issuecomment-865045209
--
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]
