Copilot commented on code in PR #66979:
URL: https://github.com/apache/airflow/pull/66979#discussion_r3246265557
##########
providers/google/src/airflow/providers/google/cloud/operators/vertex_ai/ray.py:
##########
@@ -155,7 +155,7 @@ def __init__(
**kwargs,
) -> None:
super().__init__(*args, **kwargs)
- self.head_node_type = head_node_type
+ self.head_node_type = head_node_type if head_node_type is not None
else resources.Resources()
Review Comment:
The new default path for omitted `head_node_type` is not covered by the
existing Ray operator tests; they only pass an explicit resource. Please add a
regression test that constructs or executes the operator without
`head_node_type` and verifies a fresh default `Resources` is passed through, so
this mutable-default fix cannot regress.
##########
providers/google/src/airflow/providers/google/cloud/hooks/vertex_ai/ray.py:
##########
@@ -115,7 +115,7 @@ def create_ray_cluster(
"""
aiplatform.init(project=project_id, location=location,
credentials=self.get_credentials())
cluster_path = vertex_ray.create_ray_cluster(
- head_node_type=head_node_type,
+ head_node_type=head_node_type if head_node_type is not None else
resources.Resources(),
Review Comment:
The hook's omitted-`head_node_type` branch is new behavior but the current
hook tests always pass `TEST_NODE_RESOURCES`. Please add a regression test for
calling `create_ray_cluster` without `head_node_type` and assert that a default
`Resources` instance is supplied to `vertex_ray.create_ray_cluster`.
--
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]
