eladkal commented on code in PR #53182:
URL: https://github.com/apache/airflow/pull/53182#discussion_r2201157704
##########
providers/google/src/airflow/providers/google/cloud/operators/vertex_ai/ray.py:
##########
@@ -136,9 +138,9 @@ class CreateRayClusterOperator(RayBaseOperator):
def __init__(
self,
+ python_version: str,
+ ray_version: Literal["2.9.3", "2.33", "2.42"],
Review Comment:
> But maybe I am guessing what really the problem you have is wrong.. If
your problem is the "list" itself - that sounds like property of the vertexai
service and unless we can get a constant from vertex ai, I guess we have no
other choice but to have the hard-coded list.
Exactly, we should not maintain it from our side.
We should just submit whatever value users put there if the vertexai api is
happy with it, we should be happy with it.
> Or do you propose not have the list at all and let the users find out that
they put the wrong version after the ray job is already submitted ?
I'm not sure I understand? This is a judgment call but generally we do not
do such validations we submit the values. It's up to the upstream library (SDK
for the API) to put validations should they want to before they process the API
call. Airflow should not be the gateway for parameters validations. We had
several Github issues reported on problems that came from doing such validation
- it creates issues when upstream allows a value but Airflow blocks it.
> If google team is ok and confirm that it works - I have completely no
problem with it.
Yes but I am also reflecting the side of bug reports. Google is not doing
the triage.
And also, we had a case in another provider where i had to cut ad-hoc
release due to limitation we had in our code that was no longer relevant for
the SDK - so I am also thinking about reducing ad-hoc/"emergency" releases.
None of us wants to have a case of end user that is blocked due to Airflow side.
Note I marked my comments as comments not as "request changes". I am not
blocking this merge and if you feel the current code is the right approach feel
free to proceed with merge :)
--
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]
