Skip to content

Conversation

@lidalei
Copy link
Contributor

@lidalei lidalei commented Nov 5, 2022

This PR fixes an issue when canceling BigQuery jobs. The issue arises when the service account is in a Google Cloud project A but the job runs in another project B. The default project and location won't work anymore.

@lidalei lidalei requested a review from turbaszek as a code owner November 5, 2022 20:35
@boring-cyborg boring-cyborg bot added area:providers provider:google Google (including GCP) related issues labels Nov 5, 2022
Copy link
Contributor

@o-nikolas o-nikolas left a comment

Choose a reason for hiding this comment

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

Nice catch 👍

@uranusjr
Copy link
Member

uranusjr commented Nov 7, 2022

Should we have a test for this?

@lidalei lidalei changed the title set project_id and location when cancelijng bigquery job set project_id and location when canceling bigquery job Nov 7, 2022
@lidalei lidalei changed the title set project_id and location when canceling bigquery job set project_id and location when canceling BigQuery job Nov 7, 2022
self.hook.cancel_query()

calls = [mock.call(job_id=running_job_id), mock.call(running_job_id)]
calls = [
Copy link
Contributor Author

@lidalei lidalei Nov 7, 2022

Choose a reason for hiding this comment

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

@uranusjr Thx for your comment. The two test cases test cancel_query which calls cancel_job. They also check the calls invoking poll_job_complete. So I think it is good enough. During test, I need to set an environment variable AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT=google-cloud-platform:// because the GoogleBaseHook calls get_connection in its constructor. I think we should move that function call within a member function. But it is out of the scope of this PR.

One concern is that cancel_query is being deprecated. When we fully deprecate it, we shall convert some of the test cases to cover cancel_job.

@uranusjr uranusjr merged commit 98a9c57 into apache:main Nov 7, 2022
@lidalei lidalei deleted the fix_bq_job_nont_found_on_kill branch November 8, 2022 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants