ahidalgob commented on code in PR #33067: URL: https://github.com/apache/airflow/pull/33067#discussion_r1295465701
########## airflow/providers/google/cloud/hooks/cloud_run.py: ########## @@ -0,0 +1,184 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import itertools +import json +from typing import Iterable, Sequence + +from google.api_core import operation +from google.cloud.run_v2 import ( + CreateJobRequest, + DeleteJobRequest, + GetJobRequest, + Job, + JobsAsyncClient, + JobsClient, + ListJobsRequest, + RunJobRequest, + UpdateJobRequest, +) +from google.cloud.run_v2.services.jobs import pagers +from google.longrunning import operations_pb2 + +from airflow.exceptions import AirflowException +from airflow.providers.google.common.consts import CLIENT_INFO +from airflow.providers.google.common.hooks.base_google import PROVIDE_PROJECT_ID, GoogleBaseHook + + +class CloudRunHook(GoogleBaseHook): + """ + Hook for the Google Cloud Run service. + :param gcp_conn_id: The connection ID to use when fetching connection info. + :param impersonation_chain: Optional service account to impersonate using short-term + credentials, or chained list of accounts required to get the access_token + of the last account in the list, which will be impersonated in the request. + If set as a string, the account must grant the originating account + the Service Account Token Creator IAM role. + If set as a sequence, the identities from the list must grant + Service Account Token Creator IAM role to the directly preceding identity, with first + account from the list granting this role to the originating account. + """ + + def __init__( + self, + gcp_conn_id: str = "google_cloud_default", + impersonation_chain: str | Sequence[str] | None = None, + **kwargs, + ) -> None: + if kwargs.get("delegate_to") is not None: + raise RuntimeError( + "The `delegate_to` parameter has been deprecated before and finally removed in this version" + " of Google Provider. You MUST convert it to `impersonate_chain`" + ) + super().__init__(gcp_conn_id=gcp_conn_id, impersonation_chain=impersonation_chain) + self._client: JobsClient | None = None + + def get_conn(self) -> JobsClient: + """ + Retrieves connection to Cloud Run. + :return: JobsClient. + """ + if self._client is None: + self._client = JobsClient(credentials=self.get_credentials(), client_info=CLIENT_INFO) + return self._client + + @GoogleBaseHook.fallback_to_default_project_id + def delete_job(self, job_name: str, region: str, project_id: str = PROVIDE_PROJECT_ID) -> Job: + + create_request = DeleteJobRequest() Review Comment: ```suggestion delete_request = DeleteJobRequest() ``` ########## airflow/providers/google/cloud/hooks/cloud_run.py: ########## @@ -0,0 +1,184 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import itertools +import json +from typing import Iterable, Sequence + +from google.api_core import operation +from google.cloud.run_v2 import ( + CreateJobRequest, + DeleteJobRequest, + GetJobRequest, + Job, + JobsAsyncClient, + JobsClient, + ListJobsRequest, + RunJobRequest, + UpdateJobRequest, +) +from google.cloud.run_v2.services.jobs import pagers +from google.longrunning import operations_pb2 + +from airflow.exceptions import AirflowException +from airflow.providers.google.common.consts import CLIENT_INFO +from airflow.providers.google.common.hooks.base_google import PROVIDE_PROJECT_ID, GoogleBaseHook + + +class CloudRunHook(GoogleBaseHook): + """ + Hook for the Google Cloud Run service. + :param gcp_conn_id: The connection ID to use when fetching connection info. + :param impersonation_chain: Optional service account to impersonate using short-term + credentials, or chained list of accounts required to get the access_token + of the last account in the list, which will be impersonated in the request. + If set as a string, the account must grant the originating account + the Service Account Token Creator IAM role. + If set as a sequence, the identities from the list must grant + Service Account Token Creator IAM role to the directly preceding identity, with first + account from the list granting this role to the originating account. + """ + + def __init__( + self, + gcp_conn_id: str = "google_cloud_default", + impersonation_chain: str | Sequence[str] | None = None, + **kwargs, + ) -> None: + if kwargs.get("delegate_to") is not None: + raise RuntimeError( + "The `delegate_to` parameter has been deprecated before and finally removed in this version" + " of Google Provider. You MUST convert it to `impersonate_chain`" + ) + super().__init__(gcp_conn_id=gcp_conn_id, impersonation_chain=impersonation_chain) + self._client: JobsClient | None = None + + def get_conn(self) -> JobsClient: + """ + Retrieves connection to Cloud Run. + :return: JobsClient. + """ + if self._client is None: + self._client = JobsClient(credentials=self.get_credentials(), client_info=CLIENT_INFO) + return self._client + + @GoogleBaseHook.fallback_to_default_project_id + def delete_job(self, job_name: str, region: str, project_id: str = PROVIDE_PROJECT_ID) -> Job: + + create_request = DeleteJobRequest() + create_request.name = f"projects/{project_id}/locations/{region}/jobs/{job_name}" + + operation = self.get_conn().delete_job(create_request) + return operation.result() + + @GoogleBaseHook.fallback_to_default_project_id + def create_job(self, job_name: str, job, region: str, project_id: str = PROVIDE_PROJECT_ID) -> Job: Review Comment: ```suggestion def create_job(self, job_name: str, job: Job | dict, region: str, project_id: str = PROVIDE_PROJECT_ID) -> Job: ``` Same in `update_job` ########## docs/apache-airflow-providers-google/operators/cloud/cloud_run.rst: ########## Review Comment: In Google operators doc we include /operators/_partials/prerequisite_tasks.rst as prerequisites ########## airflow/providers/google/cloud/triggers/cloud_run.py: ########## @@ -0,0 +1,145 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import asyncio +from enum import Enum +from typing import Any, AsyncIterator, Sequence + +from google.longrunning import operations_pb2 + +from airflow.exceptions import AirflowException +from airflow.providers.google.cloud.hooks.cloud_run import CloudRunAsyncHook +from airflow.triggers.base import BaseTrigger, TriggerEvent + +DEFAULT_BATCH_LOCATION = "us-central1" + + +class RunJobStatus(Enum): + """Enum to represent the status of a job run.""" + + SUCCESS = "Success" + FAIL = "Fail" + TIMEOUT = "Timeout" + + +class CloudRunJobFinishedTrigger(BaseTrigger): + """Cloud Run trigger to check if templated job has been finished. + :param operation_name: Required. Name of the operation. + :param job_name: Required. Name of the job. + :param project_id: Required. the Google Cloud project ID in which the job was started. + :param location: Optional. the location where job is executed. + If set to None then the value of DEFAULT_BATCH_LOCATION will be used. + :param gcp_conn_id: The connection ID to use connecting to Google Cloud. + :param impersonation_chain: Optional. Service account to impersonate using short-term + credentials, or chained list of accounts required to get the access_token of the last account in the list, + which will be impersonated in the request. + If set as a string, the account must grant the originating account + the Service Account Token Creator IAM role. + If set as a sequence, the identities from the list must grant + Service Account Token Creator IAM role to the directly preceding identity, with first + account from the list granting this role to the originating account (templated). + :param poll_sleep: Polling period in seconds to check for the status. + :timeout: The time to wait before failing the operation. + """ + + def __init__( + self, + operation_name: str, + job_name: str, + project_id: str | None, + location: str = DEFAULT_BATCH_LOCATION, + gcp_conn_id: str = "google_cloud_default", + impersonation_chain: str | Sequence[str] | None = None, + polling_period_seconds: float = 10, + timeout: float | None = None, + ): + super().__init__() + self.project_id = project_id + self.job_name = job_name + self.operation_name = operation_name + self.location = location + self.gcp_conn_id = gcp_conn_id + self.polling_period_seconds = polling_period_seconds + self.timeout = timeout + self.impersonation_chain = impersonation_chain + + def serialize(self) -> tuple[str, dict[str, Any]]: + """Serializes class arguments and classpath.""" + return ( + "airflow.providers.google.cloud.triggers.cloud_run.CloudRunJobFinishedTrigger", + { + "project_id": self.project_id, + "operation_name": self.operation_name, + "job_name": self.job_name, + "location": self.location, + "gcp_conn_id": self.gcp_conn_id, + "polling_period_seconds": self.polling_period_seconds, + "timeout": self.timeout, + "impersonation_chain": self.impersonation_chain, + }, + ) + + async def run(self) -> AsyncIterator[TriggerEvent]: + """ + Main loop of the class in where it is fetching the operation status and yields an + Event when done. + + """ + timeout = self.timeout + hook = self._get_async_hook() + while timeout is None or timeout > 0: + operation: operations_pb2.Operation = await hook.get_operation(self.operation_name) + if operation.done: + # An operation can only have one of those two combinations: if it is succeeded, then + # the response field will be populated, else, then the error field will be. + if operation.response is not None: + yield TriggerEvent( + { + "status": RunJobStatus.SUCCESS, + "job_name": self.job_name, + } + ) + else: + yield TriggerEvent( + { + "status": RunJobStatus.FAIL, + "operation_error_code": operation.error.code, + "operation_error_message": operation.error.message, + "job_name": self.job_name, + } + ) + elif operation.error.message: + raise AirflowException(f"Cloud Run Job error: {operation.error.message}") + + if timeout is not None: + timeout -= self.polling_period_seconds + + await asyncio.sleep(self.polling_period_seconds) Review Comment: On the last iteration of this while loop we call sleep only to come back to exit the loop. I think it's better to only sleep if `timeout > 0` -- 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]
