shahar1 commented on code in PR #75969:
URL: https://github.com/apache/airflow/pull/75969#discussion_r4421255933


##########
providers/google/src/airflow/providers/google/cloud/operators/gen_ai.py:
##########
@@ -584,9 +593,26 @@ def hook(self):
             gcp_conn_id=self.gcp_conn_id,
             impersonation_chain=self.impersonation_chain,
             gemini_api_key=self.gemini_api_key,
+            vertexai=self.vertexai,
+            project_id=self.project_id,
+            location=self.location,
         )
 
+    def _validate_input_source(self) -> None:

Review Comment:
   nit: `_validate_input_source` also checks `gemini_api_key` and 
`results_folder`. A name like `_validate_mode_params` fits better.



##########
providers/google/src/airflow/providers/google/cloud/hooks/vertex_ai/batch_prediction_job.py:
##########
@@ -82,6 +91,17 @@ def cancel_batch_prediction_job(self) -> None:
         if self._batch_prediction_job:
             self._batch_prediction_job.cancel()
 
+    def _init_aiplatform(self, *, project_id: str, region: str, credentials: 
Credentials) -> None:
+        # BatchPredictionJob.create/submit build 
"{location}-aiplatform.googleapis.com" from their
+        # location argument, which is wrong for "global". aiplatform.init 
state is process-wide and
+        # sticky, so it is set on every call to avoid reusing the endpoint of 
a previous job.

Review Comment:
   nit: The comment says the endpoint is set on every call to avoid reusing a 
previous job's endpoint, but `aiplatform.init` ignores `api_endpoint=None`. In 
a non-default universe `_get_api_endpoint` returns `None`, so a previously set 
endpoint is kept.



##########
providers/google/src/airflow/providers/google/cloud/operators/gen_ai.py:
##########
@@ -506,6 +509,7 @@ def __init__(
         wait_until_complete: bool = False,
         polling_interval: int = 30,
         deferrable: bool = conf.getboolean("operators", "default_deferrable", 
fallback=False),
+        vertexai: bool = False,

Review Comment:
   1. I'd rename the parameter to `use_vertex_ai`, so it's clear it is a 
boolean flag (matches use_regional_endpoint and similar elsewhere in the 
provider).
   2. The sibling operators (Get/List/Cancel/Delete batch job) don't get this 
option, so a batch job created with `=True` can't be managed with them.
   3. Because of that, the system test doesn't delete the Vertex AI batch job 
after it finishes, unlike the other batch jobs in the same Dag.



-- 
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]

Reply via email to