From 027aeaf72b73088c05dc13c0c30502cb03ef2c79 Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Fri, 7 Apr 2023 11:30:36 +0900 Subject: [PATCH 01/11] Fix cloud build create build log --- airflow/providers/google/cloud/hooks/cloud_build.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/airflow/providers/google/cloud/hooks/cloud_build.py b/airflow/providers/google/cloud/hooks/cloud_build.py index 826bc6bcbebfe..7f6a6c1e78bb6 100644 --- a/airflow/providers/google/cloud/hooks/cloud_build.py +++ b/airflow/providers/google/cloud/hooks/cloud_build.py @@ -224,14 +224,13 @@ def create_build( ) id_ = self._get_build_id_from_operation(operation) + self.log.info("Build has been created: %s.", id_) if not wait: return self.get_build(id_=id_, project_id=project_id) operation.result() - self.log.info("Build has been created: %s.", id_) - return self.get_build(id_=id_, project_id=project_id) @GoogleBaseHook.fallback_to_default_project_id From bfd61e5033159cc79eb8185829d76565f0e043fe Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Fri, 7 Apr 2023 11:54:27 +0900 Subject: [PATCH 02/11] Use self.wait_for_operation --- .../providers/google/cloud/hooks/cloud_build.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/airflow/providers/google/cloud/hooks/cloud_build.py b/airflow/providers/google/cloud/hooks/cloud_build.py index 7f6a6c1e78bb6..80aec8c0d4478 100644 --- a/airflow/providers/google/cloud/hooks/cloud_build.py +++ b/airflow/providers/google/cloud/hooks/cloud_build.py @@ -185,6 +185,8 @@ def create_build_without_waiting_for_result( metadata=metadata, ) id_ = self._get_build_id_from_operation(operation) + self.log.info("Build has been created: %s.", id_) + return operation, id_ @GoogleBaseHook.fallback_to_default_project_id @@ -229,7 +231,7 @@ def create_build( if not wait: return self.get_build(id_=id_, project_id=project_id) - operation.result() + self.wait_for_operation(operation, timeout) return self.get_build(id_=id_, project_id=project_id) @@ -524,13 +526,12 @@ def retry_build( ) id_ = self._get_build_id_from_operation(operation) + self.log.info("Build has been retried: %s.", id_) if not wait: return self.get_build(id_=id_, project_id=project_id, location=location) - operation.result() - - self.log.info("Build has been retried: %s.", id_) + self.wait_for_operation(operation, timeout) return self.get_build(id_=id_, project_id=project_id, location=location) @@ -571,14 +572,15 @@ def run_build_trigger( timeout=timeout, metadata=metadata, ) + self.log.info("Build trigger has been run: %s.", trigger_id) id_ = self._get_build_id_from_operation(operation) + self.log.info("Build has been created: %s.", id_) if not wait: return self.get_build(id_=id_, project_id=project_id, location=location) - operation.result() - self.log.info("Build trigger has been run: %s.", trigger_id) + self.wait_for_operation(operation, timeout) return self.get_build(id_=id_, project_id=project_id, location=location) From ee8eefbb0987f0ffba32de8b8e29dbbbe4fdfa32 Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Fri, 7 Apr 2023 12:04:25 +0900 Subject: [PATCH 03/11] Remove unused func and fix test --- .../google/cloud/hooks/cloud_build.py | 46 ------------------- .../google/cloud/hooks/test_cloud_build.py | 40 ++++------------ 2 files changed, 9 insertions(+), 77 deletions(-) diff --git a/airflow/providers/google/cloud/hooks/cloud_build.py b/airflow/providers/google/cloud/hooks/cloud_build.py index 80aec8c0d4478..28da2b3acc719 100644 --- a/airflow/providers/google/cloud/hooks/cloud_build.py +++ b/airflow/providers/google/cloud/hooks/cloud_build.py @@ -189,52 +189,6 @@ def create_build_without_waiting_for_result( return operation, id_ - @GoogleBaseHook.fallback_to_default_project_id - def create_build( - self, - build: dict | Build, - project_id: str = PROVIDE_PROJECT_ID, - wait: bool = True, - retry: Retry | _MethodDefault = DEFAULT, - timeout: float | None = None, - metadata: Sequence[tuple[str, str]] = (), - ) -> Build: - """ - Starts a build with the specified configuration. - - :param build: The build resource to create. If a dict is provided, it must be of the same form - as the protobuf message `google.cloud.devtools.cloudbuild_v1.types.Build` - :param project_id: Optional, Google Cloud Project project_id where the function belongs. - If set to None or missing, the default project_id from the GCP connection is used. - :param wait: Optional, wait for operation to finish. - :param retry: Optional, a retry object used to retry requests. If `None` is specified, requests - will not be retried. - :param timeout: Optional, the amount of time, in seconds, to wait for the request to complete. - Note that if `retry` is specified, the timeout applies to each individual attempt. - :param metadata: Optional, additional metadata that is provided to the method. - - """ - client = self.get_conn() - - self.log.info("Start creating build...") - - operation = client.create_build( - request={"project_id": project_id, "build": build}, - retry=retry, - timeout=timeout, - metadata=metadata, - ) - - id_ = self._get_build_id_from_operation(operation) - self.log.info("Build has been created: %s.", id_) - - if not wait: - return self.get_build(id_=id_, project_id=project_id) - - self.wait_for_operation(operation, timeout) - - return self.get_build(id_=id_, project_id=project_id) - @GoogleBaseHook.fallback_to_default_project_id def create_build_trigger( self, diff --git a/tests/providers/google/cloud/hooks/test_cloud_build.py b/tests/providers/google/cloud/hooks/test_cloud_build.py index 01ea91d1bf7fb..c854a1416e4fc 100644 --- a/tests/providers/google/cloud/hooks/test_cloud_build.py +++ b/tests/providers/google/cloud/hooks/test_cloud_build.py @@ -90,44 +90,22 @@ def test_cancel_build(self, get_conn): @mock.patch( "airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook._get_build_id_from_operation" ) - @mock.patch("airflow.providers.google.cloud.hooks.cloud_build.TIME_TO_SLEEP_IN_SECONDS") @mock.patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook.get_conn") - def test_create_build_with_wait(self, get_conn, wait_time, mock_get_id_from_operation): + def test_create_build_without_waiting_for_result(self, get_conn, mock_get_id_from_operation): get_conn.return_value.run_build_trigger.return_value = mock.MagicMock() mock_get_id_from_operation.return_value = BUILD_ID - wait_time.return_value = 0 - - self.hook.create_build(build=BUILD, project_id=PROJECT_ID) - - get_conn.return_value.create_build.assert_called_once_with( - request={"project_id": PROJECT_ID, "build": BUILD}, retry=DEFAULT, timeout=None, metadata=() - ) - - get_conn.return_value.create_build.return_value.result.assert_called_once_with() - - get_conn.return_value.get_build.assert_called_once_with( - request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() + self.hook.create_build_without_waiting_for_result( + build=BUILD, project_id=PROJECT_ID, location=LOCATION ) - @mock.patch( - "airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook._get_build_id_from_operation" - ) - @mock.patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook.get_conn") - def test_create_build_without_wait(self, get_conn, mock_get_id_from_operation): - get_conn.return_value.run_build_trigger.return_value = mock.MagicMock() - mock_get_id_from_operation.return_value = BUILD_ID - - self.hook.create_build(build=BUILD, project_id=PROJECT_ID, wait=False) - mock_operation = get_conn.return_value.create_build mock_operation.assert_called_once_with( - request={"project_id": PROJECT_ID, "build": BUILD}, retry=DEFAULT, timeout=None, metadata=() - ) - - get_conn.return_value.get_build.assert_called_once_with( - request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() + request={"parent": PARENT, "project_id": PROJECT_ID, "build": BUILD}, + retry=DEFAULT, + timeout=None, + metadata=(), ) mock_get_id_from_operation.assert_called_once_with(mock_operation()) @@ -220,7 +198,7 @@ def test_retry_build_with_wait(self, get_conn, wait_time, mock_get_id_from_opera request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() ) - get_conn.return_value.retry_build.return_value.result.assert_called_once_with() + get_conn.return_value.retry_build.return_value.result.assert_called_once_with(timeout=None) get_conn.return_value.get_build.assert_called_once_with( request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() @@ -274,7 +252,7 @@ def test_run_build_trigger_with_wait(self, get_conn, wait_time, mock_get_id_from metadata=(), ) - get_conn.return_value.run_build_trigger.return_value.result.assert_called_once_with() + get_conn.return_value.run_build_trigger.return_value.result.assert_called_once_with(timeout=None) get_conn.return_value.get_build.assert_called_once_with( request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() From 6705da248c3759fec92f3d5ee22c41e251c6c466 Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Fri, 7 Apr 2023 12:18:52 +0900 Subject: [PATCH 04/11] Delete duplicate test --- tests/providers/google/cloud/operators/test_cloud_build.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/providers/google/cloud/operators/test_cloud_build.py b/tests/providers/google/cloud/operators/test_cloud_build.py index bd02f3e02e8be..ea8f20abcb015 100644 --- a/tests/providers/google/cloud/operators/test_cloud_build.py +++ b/tests/providers/google/cloud/operators/test_cloud_build.py @@ -475,13 +475,6 @@ def test_async_create_build_error_event_should_throw_exception(): operator.execute_complete(context=None, event={"status": "error", "message": "test failure message"}) -@mock.patch(CLOUD_BUILD_HOOK_PATH) -def test_async_create_build_with_missing_build_should_throw_exception(mock_hook): - mock_hook.return_value.create_build.return_value = Build() - with pytest.raises(AirflowException, match="missing keyword argument 'build'"): - CloudBuildCreateBuildOperator(task_id="id") - - @pytest.mark.parametrize( "file_type, file_content", [ From 22c922ab88086c80c2c83e83599bda6793cc968e Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Sun, 16 Apr 2023 11:36:56 +0900 Subject: [PATCH 05/11] Revert "Delete duplicate test" This reverts commit 6705da248c3759fec92f3d5ee22c41e251c6c466. --- tests/providers/google/cloud/operators/test_cloud_build.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/providers/google/cloud/operators/test_cloud_build.py b/tests/providers/google/cloud/operators/test_cloud_build.py index ea8f20abcb015..bd02f3e02e8be 100644 --- a/tests/providers/google/cloud/operators/test_cloud_build.py +++ b/tests/providers/google/cloud/operators/test_cloud_build.py @@ -475,6 +475,13 @@ def test_async_create_build_error_event_should_throw_exception(): operator.execute_complete(context=None, event={"status": "error", "message": "test failure message"}) +@mock.patch(CLOUD_BUILD_HOOK_PATH) +def test_async_create_build_with_missing_build_should_throw_exception(mock_hook): + mock_hook.return_value.create_build.return_value = Build() + with pytest.raises(AirflowException, match="missing keyword argument 'build'"): + CloudBuildCreateBuildOperator(task_id="id") + + @pytest.mark.parametrize( "file_type, file_content", [ From d73b32c733e3dcaf8e27d868c2b5c59c5d68a872 Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Sun, 16 Apr 2023 11:37:10 +0900 Subject: [PATCH 06/11] Revert "Remove unused func and fix test" This reverts commit ee8eefbb0987f0ffba32de8b8e29dbbbe4fdfa32. --- .../google/cloud/hooks/cloud_build.py | 46 +++++++++++++++++++ .../google/cloud/hooks/test_cloud_build.py | 40 ++++++++++++---- 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/airflow/providers/google/cloud/hooks/cloud_build.py b/airflow/providers/google/cloud/hooks/cloud_build.py index 28da2b3acc719..80aec8c0d4478 100644 --- a/airflow/providers/google/cloud/hooks/cloud_build.py +++ b/airflow/providers/google/cloud/hooks/cloud_build.py @@ -189,6 +189,52 @@ def create_build_without_waiting_for_result( return operation, id_ + @GoogleBaseHook.fallback_to_default_project_id + def create_build( + self, + build: dict | Build, + project_id: str = PROVIDE_PROJECT_ID, + wait: bool = True, + retry: Retry | _MethodDefault = DEFAULT, + timeout: float | None = None, + metadata: Sequence[tuple[str, str]] = (), + ) -> Build: + """ + Starts a build with the specified configuration. + + :param build: The build resource to create. If a dict is provided, it must be of the same form + as the protobuf message `google.cloud.devtools.cloudbuild_v1.types.Build` + :param project_id: Optional, Google Cloud Project project_id where the function belongs. + If set to None or missing, the default project_id from the GCP connection is used. + :param wait: Optional, wait for operation to finish. + :param retry: Optional, a retry object used to retry requests. If `None` is specified, requests + will not be retried. + :param timeout: Optional, the amount of time, in seconds, to wait for the request to complete. + Note that if `retry` is specified, the timeout applies to each individual attempt. + :param metadata: Optional, additional metadata that is provided to the method. + + """ + client = self.get_conn() + + self.log.info("Start creating build...") + + operation = client.create_build( + request={"project_id": project_id, "build": build}, + retry=retry, + timeout=timeout, + metadata=metadata, + ) + + id_ = self._get_build_id_from_operation(operation) + self.log.info("Build has been created: %s.", id_) + + if not wait: + return self.get_build(id_=id_, project_id=project_id) + + self.wait_for_operation(operation, timeout) + + return self.get_build(id_=id_, project_id=project_id) + @GoogleBaseHook.fallback_to_default_project_id def create_build_trigger( self, diff --git a/tests/providers/google/cloud/hooks/test_cloud_build.py b/tests/providers/google/cloud/hooks/test_cloud_build.py index c854a1416e4fc..01ea91d1bf7fb 100644 --- a/tests/providers/google/cloud/hooks/test_cloud_build.py +++ b/tests/providers/google/cloud/hooks/test_cloud_build.py @@ -90,22 +90,44 @@ def test_cancel_build(self, get_conn): @mock.patch( "airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook._get_build_id_from_operation" ) + @mock.patch("airflow.providers.google.cloud.hooks.cloud_build.TIME_TO_SLEEP_IN_SECONDS") @mock.patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook.get_conn") - def test_create_build_without_waiting_for_result(self, get_conn, mock_get_id_from_operation): + def test_create_build_with_wait(self, get_conn, wait_time, mock_get_id_from_operation): get_conn.return_value.run_build_trigger.return_value = mock.MagicMock() mock_get_id_from_operation.return_value = BUILD_ID - self.hook.create_build_without_waiting_for_result( - build=BUILD, project_id=PROJECT_ID, location=LOCATION + wait_time.return_value = 0 + + self.hook.create_build(build=BUILD, project_id=PROJECT_ID) + + get_conn.return_value.create_build.assert_called_once_with( + request={"project_id": PROJECT_ID, "build": BUILD}, retry=DEFAULT, timeout=None, metadata=() + ) + + get_conn.return_value.create_build.return_value.result.assert_called_once_with() + + get_conn.return_value.get_build.assert_called_once_with( + request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() ) + @mock.patch( + "airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook._get_build_id_from_operation" + ) + @mock.patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook.get_conn") + def test_create_build_without_wait(self, get_conn, mock_get_id_from_operation): + get_conn.return_value.run_build_trigger.return_value = mock.MagicMock() + mock_get_id_from_operation.return_value = BUILD_ID + + self.hook.create_build(build=BUILD, project_id=PROJECT_ID, wait=False) + mock_operation = get_conn.return_value.create_build mock_operation.assert_called_once_with( - request={"parent": PARENT, "project_id": PROJECT_ID, "build": BUILD}, - retry=DEFAULT, - timeout=None, - metadata=(), + request={"project_id": PROJECT_ID, "build": BUILD}, retry=DEFAULT, timeout=None, metadata=() + ) + + get_conn.return_value.get_build.assert_called_once_with( + request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() ) mock_get_id_from_operation.assert_called_once_with(mock_operation()) @@ -198,7 +220,7 @@ def test_retry_build_with_wait(self, get_conn, wait_time, mock_get_id_from_opera request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() ) - get_conn.return_value.retry_build.return_value.result.assert_called_once_with(timeout=None) + get_conn.return_value.retry_build.return_value.result.assert_called_once_with() get_conn.return_value.get_build.assert_called_once_with( request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() @@ -252,7 +274,7 @@ def test_run_build_trigger_with_wait(self, get_conn, wait_time, mock_get_id_from metadata=(), ) - get_conn.return_value.run_build_trigger.return_value.result.assert_called_once_with(timeout=None) + get_conn.return_value.run_build_trigger.return_value.result.assert_called_once_with() get_conn.return_value.get_build.assert_called_once_with( request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() From 51a5b0cec33c2865563a22aed917203e8c39769a Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Sun, 16 Apr 2023 11:47:13 +0900 Subject: [PATCH 07/11] Add test_create_build_with_wait --- .../google/cloud/hooks/cloud_build.py | 5 ++-- .../google/cloud/hooks/test_cloud_build.py | 27 +++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/airflow/providers/google/cloud/hooks/cloud_build.py b/airflow/providers/google/cloud/hooks/cloud_build.py index 80aec8c0d4478..c904b73486d1b 100644 --- a/airflow/providers/google/cloud/hooks/cloud_build.py +++ b/airflow/providers/google/cloud/hooks/cloud_build.py @@ -226,12 +226,13 @@ def create_build( ) id_ = self._get_build_id_from_operation(operation) - self.log.info("Build has been created: %s.", id_) if not wait: return self.get_build(id_=id_, project_id=project_id) - self.wait_for_operation(operation, timeout) + operation.result() + + self.log.info("Build has been created: %s.", id_) return self.get_build(id_=id_, project_id=project_id) diff --git a/tests/providers/google/cloud/hooks/test_cloud_build.py b/tests/providers/google/cloud/hooks/test_cloud_build.py index 01ea91d1bf7fb..bc21aad414d3c 100644 --- a/tests/providers/google/cloud/hooks/test_cloud_build.py +++ b/tests/providers/google/cloud/hooks/test_cloud_build.py @@ -110,6 +110,29 @@ def test_create_build_with_wait(self, get_conn, wait_time, mock_get_id_from_oper request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() ) + @mock.patch( + "airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook._get_build_id_from_operation" + ) + @mock.patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook.get_conn") + def test_create_build_without_waiting_for_result(self, get_conn, mock_get_id_from_operation): + get_conn.return_value.run_build_trigger.return_value = mock.MagicMock() + mock_get_id_from_operation.return_value = BUILD_ID + + self.hook.create_build_without_waiting_for_result( + build=BUILD, project_id=PROJECT_ID, location=LOCATION + ) + + mock_operation = get_conn.return_value.create_build + + mock_operation.assert_called_once_with( + request={"parent": PARENT, "project_id": PROJECT_ID, "build": BUILD}, + retry=DEFAULT, + timeout=None, + metadata=(), + ) + + mock_get_id_from_operation.assert_called_once_with(mock_operation()) + @mock.patch( "airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook._get_build_id_from_operation" ) @@ -220,7 +243,7 @@ def test_retry_build_with_wait(self, get_conn, wait_time, mock_get_id_from_opera request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() ) - get_conn.return_value.retry_build.return_value.result.assert_called_once_with() + get_conn.return_value.retry_build.return_value.result.assert_called_once_with(timeout=None) get_conn.return_value.get_build.assert_called_once_with( request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() @@ -274,7 +297,7 @@ def test_run_build_trigger_with_wait(self, get_conn, wait_time, mock_get_id_from metadata=(), ) - get_conn.return_value.run_build_trigger.return_value.result.assert_called_once_with() + get_conn.return_value.run_build_trigger.return_value.result.assert_called_once_with(timeout=None) get_conn.return_value.get_build.assert_called_once_with( request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() From e1cd56d53d28c34d1a23f938bd037f92dba2e338 Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Sun, 23 Apr 2023 19:58:42 +0900 Subject: [PATCH 08/11] Deprecate create_build --- airflow/providers/google/cloud/hooks/cloud_build.py | 5 +++++ tests/providers/google/cloud/hooks/test_cloud_build.py | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/airflow/providers/google/cloud/hooks/cloud_build.py b/airflow/providers/google/cloud/hooks/cloud_build.py index c904b73486d1b..3dbacdbc0ee8a 100644 --- a/airflow/providers/google/cloud/hooks/cloud_build.py +++ b/airflow/providers/google/cloud/hooks/cloud_build.py @@ -214,6 +214,11 @@ def create_build( :param metadata: Optional, additional metadata that is provided to the method. """ + warnings.warn( + "This method is deprecated. Please use `create_build_without_waiting_for_result`.", + DeprecationWarning, + stacklevel=2, + ) client = self.get_conn() self.log.info("Start creating build...") diff --git a/tests/providers/google/cloud/hooks/test_cloud_build.py b/tests/providers/google/cloud/hooks/test_cloud_build.py index bc21aad414d3c..40dfce43ffc4b 100644 --- a/tests/providers/google/cloud/hooks/test_cloud_build.py +++ b/tests/providers/google/cloud/hooks/test_cloud_build.py @@ -98,7 +98,8 @@ def test_create_build_with_wait(self, get_conn, wait_time, mock_get_id_from_oper wait_time.return_value = 0 - self.hook.create_build(build=BUILD, project_id=PROJECT_ID) + with pytest.warns(DeprecationWarning, match="This method is deprecated"): + self.hook.create_build(build=BUILD, project_id=PROJECT_ID) get_conn.return_value.create_build.assert_called_once_with( request={"project_id": PROJECT_ID, "build": BUILD}, retry=DEFAULT, timeout=None, metadata=() @@ -141,7 +142,8 @@ def test_create_build_without_wait(self, get_conn, mock_get_id_from_operation): get_conn.return_value.run_build_trigger.return_value = mock.MagicMock() mock_get_id_from_operation.return_value = BUILD_ID - self.hook.create_build(build=BUILD, project_id=PROJECT_ID, wait=False) + with pytest.warns(DeprecationWarning, match="This method is deprecated"): + self.hook.create_build(build=BUILD, project_id=PROJECT_ID, wait=False) mock_operation = get_conn.return_value.create_build From 7a2deeef275cec3da89853bccf808f161410717b Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Sun, 23 Apr 2023 19:59:11 +0900 Subject: [PATCH 09/11] Refactoring test code --- .../google/cloud/hooks/test_cloud_build.py | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/providers/google/cloud/hooks/test_cloud_build.py b/tests/providers/google/cloud/hooks/test_cloud_build.py index 40dfce43ffc4b..0af3f0d1bd04b 100644 --- a/tests/providers/google/cloud/hooks/test_cloud_build.py +++ b/tests/providers/google/cloud/hooks/test_cloud_build.py @@ -115,21 +115,21 @@ def test_create_build_with_wait(self, get_conn, wait_time, mock_get_id_from_oper "airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook._get_build_id_from_operation" ) @mock.patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook.get_conn") - def test_create_build_without_waiting_for_result(self, get_conn, mock_get_id_from_operation): + def test_create_build_without_wait(self, get_conn, mock_get_id_from_operation): get_conn.return_value.run_build_trigger.return_value = mock.MagicMock() mock_get_id_from_operation.return_value = BUILD_ID - self.hook.create_build_without_waiting_for_result( - build=BUILD, project_id=PROJECT_ID, location=LOCATION - ) + with pytest.warns(DeprecationWarning, match="This method is deprecated"): + self.hook.create_build(build=BUILD, project_id=PROJECT_ID, wait=False) mock_operation = get_conn.return_value.create_build mock_operation.assert_called_once_with( - request={"parent": PARENT, "project_id": PROJECT_ID, "build": BUILD}, - retry=DEFAULT, - timeout=None, - metadata=(), + request={"project_id": PROJECT_ID, "build": BUILD}, retry=DEFAULT, timeout=None, metadata=() + ) + + get_conn.return_value.get_build.assert_called_once_with( + request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() ) mock_get_id_from_operation.assert_called_once_with(mock_operation()) @@ -138,21 +138,21 @@ def test_create_build_without_waiting_for_result(self, get_conn, mock_get_id_fro "airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook._get_build_id_from_operation" ) @mock.patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook.get_conn") - def test_create_build_without_wait(self, get_conn, mock_get_id_from_operation): + def test_create_build_without_waiting_for_result(self, get_conn, mock_get_id_from_operation): get_conn.return_value.run_build_trigger.return_value = mock.MagicMock() mock_get_id_from_operation.return_value = BUILD_ID - with pytest.warns(DeprecationWarning, match="This method is deprecated"): - self.hook.create_build(build=BUILD, project_id=PROJECT_ID, wait=False) + self.hook.create_build_without_waiting_for_result( + build=BUILD, project_id=PROJECT_ID, location=LOCATION + ) mock_operation = get_conn.return_value.create_build mock_operation.assert_called_once_with( - request={"project_id": PROJECT_ID, "build": BUILD}, retry=DEFAULT, timeout=None, metadata=() - ) - - get_conn.return_value.get_build.assert_called_once_with( - request={"project_id": PROJECT_ID, "id": BUILD_ID}, retry=DEFAULT, timeout=None, metadata=() + request={"parent": PARENT, "project_id": PROJECT_ID, "build": BUILD}, + retry=DEFAULT, + timeout=None, + metadata=(), ) mock_get_id_from_operation.assert_called_once_with(mock_operation()) From ec567a739acefafa1a5c9f20d384dce5764d8d4b Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Fri, 5 May 2023 15:12:52 +0900 Subject: [PATCH 10/11] Use AirflowProviderDeprecationWarning --- airflow/providers/google/cloud/hooks/cloud_build.py | 4 ++-- tests/providers/google/cloud/hooks/test_cloud_build.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/airflow/providers/google/cloud/hooks/cloud_build.py b/airflow/providers/google/cloud/hooks/cloud_build.py index 1200ebe4289b6..0b164a4cc2573 100644 --- a/airflow/providers/google/cloud/hooks/cloud_build.py +++ b/airflow/providers/google/cloud/hooks/cloud_build.py @@ -28,7 +28,7 @@ from google.cloud.devtools.cloudbuild_v1 import CloudBuildAsyncClient, CloudBuildClient, GetBuildRequest from google.cloud.devtools.cloudbuild_v1.types import Build, BuildTrigger, RepoSource -from airflow.exceptions import AirflowException +from airflow.exceptions import AirflowException, AirflowProviderDeprecationWarning from airflow.providers.google.common.consts import CLIENT_INFO from airflow.providers.google.common.hooks.base_google import PROVIDE_PROJECT_ID, GoogleBaseHook @@ -211,7 +211,7 @@ def create_build( """ warnings.warn( "This method is deprecated. Please use `create_build_without_waiting_for_result`.", - DeprecationWarning, + AirflowProviderDeprecationWarning, stacklevel=2, ) client = self.get_conn() diff --git a/tests/providers/google/cloud/hooks/test_cloud_build.py b/tests/providers/google/cloud/hooks/test_cloud_build.py index 823807c5dab54..a6ef382ea94de 100644 --- a/tests/providers/google/cloud/hooks/test_cloud_build.py +++ b/tests/providers/google/cloud/hooks/test_cloud_build.py @@ -28,7 +28,7 @@ from google.api_core.gapic_v1.method import DEFAULT from google.cloud.devtools.cloudbuild_v1 import CloudBuildAsyncClient, GetBuildRequest -from airflow import AirflowException +from airflow import AirflowException, AirflowProviderDeprecationWarning from airflow.providers.google.cloud.hooks.cloud_build import CloudBuildAsyncHook, CloudBuildHook from airflow.providers.google.common.consts import CLIENT_INFO from tests.providers.google.cloud.utils.base_gcp_mock import mock_base_gcp_hook_no_default_project_id @@ -102,7 +102,7 @@ def test_create_build_with_wait(self, get_conn, wait_time, mock_get_id_from_oper wait_time.return_value = 0 - with pytest.warns(DeprecationWarning, match="This method is deprecated"): + with pytest.warns(AirflowProviderDeprecationWarning, match="This method is deprecated"): self.hook.create_build(build=BUILD, project_id=PROJECT_ID) get_conn.return_value.create_build.assert_called_once_with( @@ -123,7 +123,7 @@ def test_create_build_without_wait(self, get_conn, mock_get_id_from_operation): get_conn.return_value.run_build_trigger.return_value = mock.MagicMock() mock_get_id_from_operation.return_value = BUILD_ID - with pytest.warns(DeprecationWarning, match="This method is deprecated"): + with pytest.warns(AirflowProviderDeprecationWarning, match="This method is deprecated"): self.hook.create_build(build=BUILD, project_id=PROJECT_ID, wait=False) mock_operation = get_conn.return_value.create_build From 16a1e7de840b8399f33d9ac3dae8433467f29823 Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Tue, 9 May 2023 14:11:33 +0900 Subject: [PATCH 11/11] Fix import errors --- airflow/providers/google/cloud/hooks/cloud_build.py | 1 + tests/providers/google/cloud/hooks/test_cloud_build.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/airflow/providers/google/cloud/hooks/cloud_build.py b/airflow/providers/google/cloud/hooks/cloud_build.py index 0b164a4cc2573..0ac929a4ad2fb 100644 --- a/airflow/providers/google/cloud/hooks/cloud_build.py +++ b/airflow/providers/google/cloud/hooks/cloud_build.py @@ -18,6 +18,7 @@ """Hook for Google Cloud Build service.""" from __future__ import annotations +import warnings from typing import Sequence from google.api_core.client_options import ClientOptions diff --git a/tests/providers/google/cloud/hooks/test_cloud_build.py b/tests/providers/google/cloud/hooks/test_cloud_build.py index a6ef382ea94de..070356ba68189 100644 --- a/tests/providers/google/cloud/hooks/test_cloud_build.py +++ b/tests/providers/google/cloud/hooks/test_cloud_build.py @@ -28,7 +28,7 @@ from google.api_core.gapic_v1.method import DEFAULT from google.cloud.devtools.cloudbuild_v1 import CloudBuildAsyncClient, GetBuildRequest -from airflow import AirflowException, AirflowProviderDeprecationWarning +from airflow.exceptions import AirflowException, AirflowProviderDeprecationWarning from airflow.providers.google.cloud.hooks.cloud_build import CloudBuildAsyncHook, CloudBuildHook from airflow.providers.google.common.consts import CLIENT_INFO from tests.providers.google.cloud.utils.base_gcp_mock import mock_base_gcp_hook_no_default_project_id