dstandish commented on code in PR #27197:
URL: https://github.com/apache/airflow/pull/27197#discussion_r1004707260
##########
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:
I am not saying we would not remove the resource parameter. That's a
separate topic. What we're doing here is, we're no longer helping users out by
converting `resources` into `container_resources`. In case user doesn't catch
this change (as evidenced by v1resourcerequirements being passed to resources)
we raise now instead of warn. But user could still in theory use `resources`
for whatever it's there for (i'm assuming that's something other than
v1resourcerequirements).
Essentially, the reason we should raise _conditionally_, based on what is
passed to resources, is precisely the same as whatever motivated us to _warn_
conditionally -- if there was a reason to do that there (and i'm assuming that
is to allow "proper" use of resources param), then there's a reason to do that
here.
And then maybe next major release we remove the check and just don't even
look at resources.
Whether resources gets chopped from airflow, that's a separate question.
--
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]
