Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CloudBuild build id log #30516

Merged
merged 13 commits into from
May 16, 2023
57 changes: 6 additions & 51 deletions airflow/providers/google/cloud/hooks/cloud_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,54 +185,9 @@ def create_build_without_waiting_for_result(
metadata=metadata,
)
id_ = self._get_build_id_from_operation(operation)
return operation, id_

@GoogleBaseHook.fallback_to_default_project_id
def create_build(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe need to deprecated it first and we should remove it only in major release

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pankajastro
Thank you for your review. I have reverted the changes based on your feedback.
If you happen to know, could you please tell me what is the common way to deprecate a method in Airflow?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends - look at similar cases where DeprecationWarning is raised - just search in our codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@potiuk
Thank you as always!
I changed create_build to be deprecated as it is an unused old method.
e1cd56d

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)

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)
return operation, id_

@GoogleBaseHook.fallback_to_default_project_id
def create_build_trigger(
Expand Down Expand Up @@ -525,13 +480,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)

Expand Down Expand Up @@ -572,14 +526,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)

Expand Down
40 changes: 9 additions & 31 deletions tests/providers/google/cloud/hooks/test_cloud_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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=()
Expand Down Expand Up @@ -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=()
Expand Down
7 changes: 0 additions & 7 deletions tests/providers/google/cloud/operators/test_cloud_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
[
Expand Down