Skip to content

Use insert_job in the BigQueryToGCPOpertor and adjust links - #24416

Merged
potiuk merged 3 commits into
apache:mainfrom
lwyszomi:bq_operators
Jun 15, 2022
Merged

Use insert_job in the BigQueryToGCPOpertor and adjust links#24416
potiuk merged 3 commits into
apache:mainfrom
lwyszomi:bq_operators

Conversation

@lwyszomi

Copy link
Copy Markdown
Contributor

Refactor of the BigQueryToGCPOperator, now we using insert_job method not run_extract. Also adjustment for the links in the BigQueryInsertJobOperator

FYI @potiuk


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
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 a newsfragement file, named {pr_number}.significant.rst, in newsfragments.

Comment thread airflow/providers/google/cloud/operators/bigquery.py Outdated

@turbaszek turbaszek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One comment regarding persisting links. Otherwise looks good to me.

Comment thread airflow/providers/google/cloud/transfers/bigquery_to_gcs.py
conf = job["configuration"]["extract"]["sourceTable"]

try:
self.log.info(f"Executing: {configuration}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should use %-formatting in logging so the formatting is deferred until the latest possible moment.

Comment on lines +2168 to +2171
if job_type in job.to_api_repr()["configuration"]:
for table_prop in tables_prop:
if table_prop in job.to_api_repr()["configuration"][job_type]:
table = job.to_api_repr()["configuration"][job_type][table_prop]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to write job.to_api_repr()["configuration"] 3 times? Could you please introduce a variable for this value?

@potiuk

potiuk commented Jun 14, 2022

Copy link
Copy Markdown
Member

Ah. How could I missed it !

@potiuk

potiuk commented Jun 14, 2022

Copy link
Copy Markdown
Member

Yeah. I think that last comment from @turbaszek and we can merge (and release 8.0.1)
BTW. @turbaszek - we want to relase 8.0.1 as this one fixes a serious problem in Google Provider 8.0.0 - so if you are not too strong with your comment, I'd merge and release it even now.

@potiuk

potiuk commented Jun 14, 2022

Copy link
Copy Markdown
Member

Soory @lwyszomi - it TOTALLY got under the radar - not sure how, I was waiting for that PR!

@potiuk

potiuk commented Jun 14, 2022

Copy link
Copy Markdown
Member

Ah yeah. there are also few comments from @josh-fell - so @lwyszomi -> if you can address them tomorrow morning, I am happy to wait.

@lwyszomi

Copy link
Copy Markdown
Contributor Author

@potiuk I pushed fixes for @josh-fell and @turbaszek comments.

@potiuk
potiuk dismissed turbaszek’s stale review June 15, 2022 09:44

All addressed and we want to release quickly :)

@potiuk
potiuk merged commit ce50d37 into apache:main Jun 15, 2022
kosteev pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this pull request Jul 11, 2022
Cherry-picked changes from the community:
* apache/airflow#24416
* apache/airflow#23078

Unit tests executed:
https://screenshot.googleplex.com/5AJvqmrXwoUA83L

Dags executed:
https://screenshot.googleplex.com/67ABr73jTfZETb7

Change-Id: I6f4795eed4a8ecc67b0f7997f228cd92d464d004
GitOrigin-RevId: 0fa3a13797a21cd5d6c3e08d315bb93f7af4e787
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