From 7e0cffdbe97f20d22de2bf4652e4854ff50fd32a Mon Sep 17 00:00:00 2001 From: Stephen O'Kennedy Date: Tue, 22 Mar 2022 10:06:59 +0100 Subject: [PATCH 1/2] CloudBuildRunBuildTriggerOperator: 'property' object has no attribute 'build' #22398 * Fixed issue where the `_get_build_id_from_operation` was accepting the Operation class instead of an instance of operation in the `create_build` function. * Added an assertion to verify the above fix. --- airflow/providers/google/cloud/hooks/cloud_build.py | 2 +- tests/providers/google/cloud/hooks/test_cloud_build.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/airflow/providers/google/cloud/hooks/cloud_build.py b/airflow/providers/google/cloud/hooks/cloud_build.py index 5fd2b46595776..19e41c1b265f8 100644 --- a/airflow/providers/google/cloud/hooks/cloud_build.py +++ b/airflow/providers/google/cloud/hooks/cloud_build.py @@ -162,7 +162,7 @@ def create_build( metadata=metadata, ) - id_ = self._get_build_id_from_operation(Operation) + id_ = self._get_build_id_from_operation(operation) if not wait: 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 fdac4dd1898ca..be5cdf5aa8915 100644 --- a/tests/providers/google/cloud/hooks/test_cloud_build.py +++ b/tests/providers/google/cloud/hooks/test_cloud_build.py @@ -106,7 +106,9 @@ def test_create_build_without_wait(self, get_conn, mock_get_id_from_operation): self.hook.create_build(build=BUILD, project_id=PROJECT_ID, wait=False) - get_conn.return_value.create_build.assert_called_once_with( + mock_operation = get_conn.return_value.create_build + + mock_operation.assert_called_once_with( request={'project_id': PROJECT_ID, 'build': BUILD}, retry=None, timeout=None, metadata=() ) @@ -114,6 +116,8 @@ def test_create_build_without_wait(self, get_conn, mock_get_id_from_operation): request={'project_id': PROJECT_ID, 'id': BUILD_ID}, retry=None, timeout=None, metadata=() ) + mock_get_id_from_operation.assert_called_once_with(mock_operation()) + @patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook.get_conn") def test_create_build_trigger(self, get_conn): self.hook.create_build_trigger(trigger=BUILD_TRIGGER, project_id=PROJECT_ID) From 0718a93e2718d16c764fbe94693bfd5a5f438e01 Mon Sep 17 00:00:00 2001 From: Stephen O'Kennedy Date: Wed, 23 Mar 2022 09:20:00 +0100 Subject: [PATCH 2/2] Fixed bug where retry cloud build jobs and run cloud build triggers were unable to find the build id The `retry_build` and `run_build_trigger` functions in cloud_build.py were incorrectly passing the class `Operation` to the helper function `_get_build_id_from_operation` instead of the operation instance. --- airflow/providers/google/cloud/hooks/cloud_build.py | 4 ++-- .../providers/google/cloud/hooks/test_cloud_build.py | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/airflow/providers/google/cloud/hooks/cloud_build.py b/airflow/providers/google/cloud/hooks/cloud_build.py index 19e41c1b265f8..dd6653e0575d5 100644 --- a/airflow/providers/google/cloud/hooks/cloud_build.py +++ b/airflow/providers/google/cloud/hooks/cloud_build.py @@ -460,7 +460,7 @@ def retry_build( metadata=metadata, ) - id_ = self._get_build_id_from_operation(Operation) + id_ = self._get_build_id_from_operation(operation) if not wait: return self.get_build(id_=id_, project_id=project_id) @@ -510,7 +510,7 @@ def run_build_trigger( metadata=metadata, ) - id_ = self._get_build_id_from_operation(Operation) + id_ = self._get_build_id_from_operation(operation) if not wait: 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 be5cdf5aa8915..36aaf6cbff21c 100644 --- a/tests/providers/google/cloud/hooks/test_cloud_build.py +++ b/tests/providers/google/cloud/hooks/test_cloud_build.py @@ -198,7 +198,9 @@ def test_retry_build_with_wait(self, get_conn, wait_time, mock_get_id_from_opera self.hook.retry_build(id_=BUILD_ID, project_id=PROJECT_ID) - get_conn.return_value.retry_build.assert_called_once_with( + mock_operation = get_conn.return_value.retry_build + + mock_operation.assert_called_once_with( request={'project_id': PROJECT_ID, 'id': BUILD_ID}, retry=None, timeout=None, metadata=() ) @@ -208,6 +210,8 @@ 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=None, timeout=None, metadata=() ) + mock_get_id_from_operation.assert_called_once_with(mock_operation()) + @patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook._get_build_id_from_operation") @patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook.get_conn") def test_retry_build_without_wait(self, get_conn, mock_get_id_from_operation): @@ -237,7 +241,9 @@ def test_run_build_trigger_with_wait(self, get_conn, wait_time, mock_get_id_from trigger_id=TRIGGER_ID, source=REPO_SOURCE['repo_source'], project_id=PROJECT_ID ) - get_conn.return_value.run_build_trigger.assert_called_once_with( + mock_operation = get_conn.return_value.run_build_trigger + + mock_operation.assert_called_once_with( request={ 'project_id': PROJECT_ID, 'trigger_id': TRIGGER_ID, @@ -254,6 +260,8 @@ def test_run_build_trigger_with_wait(self, get_conn, wait_time, mock_get_id_from request={'project_id': PROJECT_ID, 'id': BUILD_ID}, retry=None, timeout=None, metadata=() ) + mock_get_id_from_operation.assert_called_once_with(mock_operation()) + @patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook._get_build_id_from_operation") @patch("airflow.providers.google.cloud.hooks.cloud_build.CloudBuildHook.get_conn") def test_run_build_trigger_without_wait(self, get_conn, mock_get_id_from_operation):