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

[AIRFLOW-6981] Move Google Cloud Build from Discovery API to Python Library #8575

Closed
wants to merge 26 commits into from

Conversation

ryanyuan
Copy link
Contributor

@ryanyuan ryanyuan commented Apr 27, 2020

Move Google Cloud Build from Discovery API to Python Library
Issue: #8568
Old Jira Ticket: AIRFLOW-6981


Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Unit tests coverage for changes (not needed for documentation changes)
  • Target Github ISSUE in description if exists
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

@boring-cyborg boring-cyborg bot added area:docs provider:google Google (including GCP) related issues labels Apr 27, 2020
@ryanyuan ryanyuan force-pushed the feature/cloud-build-python branch 3 times, most recently from 9e6c692 to 361a9d2 Compare April 27, 2020 09:30
…ibrary

[AIRFLOW-6981] Move Google Cloud Build from Discovery API to Python Library
@ryanyuan ryanyuan force-pushed the feature/cloud-build-python branch 2 times, most recently from 89a0d61 to 76d1028 Compare April 27, 2020 10:53
@mik-laj mik-laj self-requested a review April 27, 2020 10:59
@ryanyuan
Copy link
Contributor Author

@mik-laj Any ideas why the builds failed on Requirements 3.7 and 3.6?

@potiuk
Copy link
Member

potiuk commented Apr 27, 2020

You modified setup.py but when you do this, you need to generate requirements (both 3.6, 3.7), Just copy-pasted the error printed in the logs:

ERROR! Requirements need to be updated!

     Please generate requirements with:

           breeze generate-requirements --python 3.6

@potiuk
Copy link
Member

potiuk commented Apr 27, 2020

You can read more about it in contributing docs: https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#generating-requirement-files

@ryanyuan
Copy link
Contributor Author

Cheers @potiuk. This time it failed at Quarantined:Pg9.6,Py3.6. But the error log didn't say much. Any suggestions?

@mik-laj
Copy link
Member

mik-laj commented Apr 28, 2020

Please ignore tests in quarantine. They are flaky.

@potiuk
Copy link
Member

potiuk commented Apr 28, 2020

Yep. It's also described here: https://github.com/apache/airflow/blob/master/TESTING.rst#quarantined-tests

@potiuk
Copy link
Member

potiuk commented Apr 28, 2020

We plan to review and fix the quarantined tests in the coming weeks and make them stable.

@ryanyuan
Copy link
Contributor Author

thanks @potiuk

@turbaszek
Copy link
Member

For the exception you had, it seems you ran the example dag but it failed in the midway and you re-ran it so that it said the trigger already existed. I tried to add a try catch to return the trigger if it already exists but I am unable to find a good way to get the trigger id, which will be used in CloudBuildHook.get_build_trigger().

Should we then add note that the operator is not idempotent?

@ryanyuan
Copy link
Contributor Author

ryanyuan commented Sep 2, 2020

For the exception you had, it seems you ran the example dag but it failed in the midway and you re-ran it so that it said the trigger already existed. I tried to add a try catch to return the trigger if it already exists but I am unable to find a good way to get the trigger id, which will be used in CloudBuildHook.get_build_trigger().

Should we then add note that the operator is not idempotent?

Good idea. I've put a message in cloud_build.rst to remind the users the build ID and the trigger ID are not idempotent.

@turbaszek
Copy link
Member

I tried running a system test test_run_example_gcp_cloud_build_trigger_dag and I got:

[2020-09-02 14:36:56,913] {taskinstance.py:1169} INFO - Exporting the following env vars:
AIRFLOW_CTX_DAG_OWNER=airflow
AIRFLOW_CTX_DAG_ID=example_gcp_cloud_build_trigger
AIRFLOW_CTX_TASK_ID=run_build_trigger
AIRFLOW_CTX_EXECUTION_DATE=2020-09-01T00:00:00+00:00
AIRFLOW_CTX_DAG_RUN_ID=backfill__2020-09-01T00:00:00+00:00
[2020-09-02 14:36:56,926] {cloud_build.py:556} INFO - Start running build trigger: d4b43269-6ab6-46e3-950e-7541fc213a6d.
[2020-09-02 14:36:56,927] {taskinstance.py:1310} ERROR - Protocol message RepoSource has no "repo_source" field.
Traceback (most recent call last):
  File "/opt/airflow/airflow/models/taskinstance.py", line 1076, in _run_raw_task
    self._prepare_and_execute_task_with_callbacks(context, task)
  File "/opt/airflow/airflow/models/taskinstance.py", line 1181, in _prepare_and_execute_task_with_callbacks
    result = self._execute_task(context, task_copy)
  File "/opt/airflow/airflow/models/taskinstance.py", line 1216, in _execute_task
    result = task_copy.execute(context=context)
  File "/opt/airflow/airflow/providers/google/cloud/operators/cloud_build.py", line 778, in execute
    metadata=self.metadata,
  File "/opt/airflow/airflow/providers/google/common/hooks/base_google.py", line 373, in inner_wrapper
    return func(self, *args, **kwargs)
  File "/opt/airflow/airflow/providers/google/cloud/hooks/cloud_build.py", line 564, in run_build_trigger
    metadata=metadata,
  File "/usr/local/lib/python3.7/site-packages/google/cloud/devtools/cloudbuild_v1/gapic/cloud_build_client.py", line 946, in run_build_trigger
    project_id=project_id, trigger_id=trigger_id, source=source
ValueError: Protocol message RepoSource has no "repo_source" field.

@ryanyuan ryanyuan force-pushed the feature/cloud-build-python branch 3 times, most recently from 00fd603 to 3905a92 Compare September 4, 2020 13:00
@ryanyuan
Copy link
Contributor Author

ryanyuan commented Sep 4, 2020

I tried running a system test test_run_example_gcp_cloud_build_trigger_dag and I got:

[2020-09-02 14:36:56,913] {taskinstance.py:1169} INFO - Exporting the following env vars:
AIRFLOW_CTX_DAG_OWNER=airflow
AIRFLOW_CTX_DAG_ID=example_gcp_cloud_build_trigger
AIRFLOW_CTX_TASK_ID=run_build_trigger
AIRFLOW_CTX_EXECUTION_DATE=2020-09-01T00:00:00+00:00
AIRFLOW_CTX_DAG_RUN_ID=backfill__2020-09-01T00:00:00+00:00
[2020-09-02 14:36:56,926] {cloud_build.py:556} INFO - Start running build trigger: d4b43269-6ab6-46e3-950e-7541fc213a6d.
[2020-09-02 14:36:56,927] {taskinstance.py:1310} ERROR - Protocol message RepoSource has no "repo_source" field.
Traceback (most recent call last):
  File "/opt/airflow/airflow/models/taskinstance.py", line 1076, in _run_raw_task
    self._prepare_and_execute_task_with_callbacks(context, task)
  File "/opt/airflow/airflow/models/taskinstance.py", line 1181, in _prepare_and_execute_task_with_callbacks
    result = self._execute_task(context, task_copy)
  File "/opt/airflow/airflow/models/taskinstance.py", line 1216, in _execute_task
    result = task_copy.execute(context=context)
  File "/opt/airflow/airflow/providers/google/cloud/operators/cloud_build.py", line 778, in execute
    metadata=self.metadata,
  File "/opt/airflow/airflow/providers/google/common/hooks/base_google.py", line 373, in inner_wrapper
    return func(self, *args, **kwargs)
  File "/opt/airflow/airflow/providers/google/cloud/hooks/cloud_build.py", line 564, in run_build_trigger
    metadata=metadata,
  File "/usr/local/lib/python3.7/site-packages/google/cloud/devtools/cloudbuild_v1/gapic/cloud_build_client.py", line 946, in run_build_trigger
    project_id=project_id, trigger_id=trigger_id, source=source
ValueError: Protocol message RepoSource has no "repo_source" field.

Should be okay now.

@mik-laj
Copy link
Member

mik-laj commented Nov 2, 2020

@ryanyuan Everything looks fine to me. Can I ask for the last rebase, please? This week I will want to merge this PR. I already have it on my to-do list and I will never miss it. I apologize for the long waiting time for reviews.

@github-actions
Copy link

github-actions bot commented Nov 3, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@ryanyuan
Copy link
Contributor Author

ryanyuan commented Nov 3, 2020

@mik-laj no worries. PTAL.

@ryanyuan
Copy link
Contributor Author

@mik-laj Please let me know when you are going to merge this PR. I will do the rebase again.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Mar 19, 2021
@github-actions github-actions bot closed this Mar 25, 2021
@ryanyuan
Copy link
Contributor Author

Hi @turbaszek @mik-laj, any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider:google Google (including GCP) related issues stale Stale PRs per the .github/workflows/stale.yml policy file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants