You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "tnk-ysk (via GitHub)" <gi...@apache.org> on 2023/02/22 10:17:20 UTC
[GitHub] [airflow] tnk-ysk opened a new pull request, #29689: Add location to CloudBuildCreateBuildOperator
tnk-ysk opened a new pull request, #29689:
URL: https://github.com/apache/airflow/pull/29689
Add location to CloudBuildCreateBuildOperator,
Allow location to be changed using 'parent' in [CreateBuildRequest](https://cloud.google.com/python/docs/reference/cloudbuild/latest/google.cloud.devtools.cloudbuild_v1.types.CreateBuildRequest)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] tnk-ysk closed pull request #29689: Add location to CloudBuildCreateBuildOperator
Posted by "tnk-ysk (via GitHub)" <gi...@apache.org>.
tnk-ysk closed pull request #29689: Add location to CloudBuildCreateBuildOperator
URL: https://github.com/apache/airflow/pull/29689
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #29689: Add location to CloudBuildCreateBuildOperator
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #29689:
URL: https://github.com/apache/airflow/pull/29689#issuecomment-1439775017
Do these need tests?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #29689: Add location to CloudBuildCreateBuildOperator
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #29689:
URL: https://github.com/apache/airflow/pull/29689#discussion_r1124049541
##########
airflow/providers/google/cloud/hooks/cloud_build.py:
##########
@@ -158,13 +159,17 @@ def create_build_without_waiting_for_result(
: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.
+ :param location: Optional, The location of the project.
+ If set to None or missing, global is used.
Review Comment:
What does “global” mean here? It may be worthwhile to clarify in the docstring. (Or maybe I’m just missing context here, I’m not very familiar with the Google Cloud Platform.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #29689: Add location to CloudBuildCreateBuildOperator
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #29689:
URL: https://github.com/apache/airflow/pull/29689#issuecomment-1439759032
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] tnk-ysk commented on a diff in pull request #29689: Add location to CloudBuildCreateBuildOperator
Posted by "tnk-ysk (via GitHub)" <gi...@apache.org>.
tnk-ysk commented on code in PR #29689:
URL: https://github.com/apache/airflow/pull/29689#discussion_r1126306502
##########
airflow/providers/google/cloud/hooks/cloud_build.py:
##########
@@ -158,13 +159,17 @@ def create_build_without_waiting_for_result(
: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.
+ :param location: Optional, The location of the project.
+ If set to None or missing, global is used.
Review Comment:
I got a response from GCP support.
As a result, it turns out that the api_endpoint must be set on the client when using a private pool or a non-global default pool.
https://github.com/googleapis/python-cloudbuild/blob/9cf4f4269b55213c1f84206923d3cdc0308ecf57/samples/snippets/quickstart.py#L35-L43
Sample code
```
from google.cloud.devtools import cloudbuild_v1
client = cloudbuild_v1.CloudBuildClient(client_options={"api_endpoint":f"{region}-cloudbuild.googleapis.com:443"})
parent = f"projects/{project}/locations/{region}"
request = cloudbuild_v1.CreateBuildRequest(build={"steps":[{"name":"ubuntu", "args":["echo", "hello"]}]}, parent=parent)
operation = client.create_build(request)
print(operation.result())
```
Similarly, I've found that some operators may not work with private pools or a non-global default pool, but I'll fix that in another PR.
```
CloudBuildCancelBuildOperator
CloudBuildGetBuildOperator
CloudBuildListBuildsOperator
CloudBuildRetryBuildOperator
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] tnk-ysk commented on pull request #29689: Add location to CloudBuildCreateBuildOperator
Posted by "tnk-ysk (via GitHub)" <gi...@apache.org>.
tnk-ysk commented on PR #29689:
URL: https://github.com/apache/airflow/pull/29689#issuecomment-1440001316
@uranusjr
Thank you for your review.
As you pointed out, the test was missing, so I fixed it!
https://github.com/apache/airflow/pull/29689/commits/9886d33e628c290b89cb8a3a33898679ef747a5c
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] tnk-ysk commented on a diff in pull request #29689: Add location to CloudBuildCreateBuildOperator
Posted by "tnk-ysk (via GitHub)" <gi...@apache.org>.
tnk-ysk commented on code in PR #29689:
URL: https://github.com/apache/airflow/pull/29689#discussion_r1126312187
##########
airflow/providers/google/cloud/hooks/cloud_build.py:
##########
@@ -158,13 +159,17 @@ def create_build_without_waiting_for_result(
: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.
+ :param location: Optional, The location of the project.
+ If set to None or missing, global is used.
Review Comment:
@uranusjr @pankajastro
I fixed it, so please re-review
https://github.com/apache/airflow/pull/29689/commits/8a0b53943a392dd563c3896b3e23dc945d1c1ec7
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] tnk-ysk commented on pull request #29689: Add location to CloudBuildCreateBuildOperator
Posted by "tnk-ysk (via GitHub)" <gi...@apache.org>.
tnk-ysk commented on PR #29689:
URL: https://github.com/apache/airflow/pull/29689#issuecomment-1456476933
Integration https://github.com/apache/airflow/pull/29937
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] tnk-ysk commented on a diff in pull request #29689: Add location to CloudBuildCreateBuildOperator
Posted by "tnk-ysk (via GitHub)" <gi...@apache.org>.
tnk-ysk commented on code in PR #29689:
URL: https://github.com/apache/airflow/pull/29689#discussion_r1124902895
##########
airflow/providers/google/cloud/hooks/cloud_build.py:
##########
@@ -158,13 +159,17 @@ def create_build_without_waiting_for_result(
: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.
+ :param location: Optional, The location of the project.
+ If set to None or missing, global is used.
Review Comment:
@uranusjr
CloudBuild locations have global (non-regional) apart from general regions.
The current operator can only create builds on global which is the default.
This PR is to be able to change that.
https://cloud.google.com/sdk/gcloud/reference/builds/submit
```
--region=REGION
The region of the Cloud Build Service to use.
Must be set to a supported region name (e.g. us-central1).
If unset, builds/region, which is the default region to use when working with Cloud Build resources, is used.
If builds/region is unset, region is set to global.
```
However, when I actually specified the location with python client and tried to [create_build](https://cloud.google.com/python/docs/reference/cloudbuild/latest/google.cloud.devtools.cloudbuild_v1.services.cloud_build.CloudBuildClient#google_cloud_devtools_cloudbuild_v1_services_cloud_build_CloudBuildClient_create_build), an error occurred and I could not change it.
```
google.api_core.exceptions.InvalidArgument: 400 generic::invalid_argument: invalid value for 'parent': location "us-central1" is unsupported, service location is "global"
```
I also tried [list_builds 'parent'](https://github.com/apache/airflow/blob/f0bd45389c7799884a26d2b9c79b0498683d3e03/airflow/providers/google/cloud/hooks/cloud_build.py#L455) used in CloudBuildListBuildsOperator , but the global one was returned and didn't seem to work.
I am contacting GCP support about this issue.
Please wait for a while until the reply comes.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org