Skip to content

Refactor BigQuery operators#8858

Merged
turbaszek merged 12 commits into
apache:masterfrom
PolideaInternal:refactor-bq-operators
May 26, 2020
Merged

Refactor BigQuery operators#8858
turbaszek merged 12 commits into
apache:masterfrom
PolideaInternal:refactor-bq-operators

Conversation

@turbaszek

@turbaszek turbaszek commented May 13, 2020

Copy link
Copy Markdown
Member

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 the provider:google Google (including GCP) related issues label May 13, 2020
@turbaszek
turbaszek force-pushed the refactor-bq-operators branch from 4c3b966 to 99faa55 Compare May 13, 2020 13:26
@turbaszek
turbaszek requested a review from potiuk May 14, 2020 09:04
@turbaszek
turbaszek marked this pull request as ready for review May 14, 2020 09:04
@turbaszek
turbaszek requested a review from mik-laj May 14, 2020 09:04
@turbaszek turbaszek mentioned this pull request May 14, 2020
6 tasks
Comment thread airflow/providers/google/cloud/operators/bigquery.py Outdated
Comment thread airflow/providers/google/cloud/operators/bigquery.py Outdated
Comment thread airflow/providers/google/cloud/operators/bigquery.py Outdated
Comment thread airflow/providers/google/cloud/operators/bigquery.py Outdated
@turbaszek
turbaszek force-pushed the refactor-bq-operators branch from 99faa55 to 416365e Compare May 19, 2020 10:03

@olchas olchas left a comment

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.

LGTM

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.

Should we raise exception when user provide mutully exclussive parameters e.g. table_resource and source_uris?

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.

I agree. the behaviour is a bit undefined if wrong combination of parameters is specified. If table_resources is undefined and schema_fields are defined and source_objects and bucket are, I am not sure what happens here. I think all the "superfluous" combinations where same data is provided by different means should result in exception

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've added an exception

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

@potiuk potiuk 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.

Few small comments

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.

By when will it be deprecated ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No idea, we've never discussed when we will deprecate such things

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

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.

I agree. the behaviour is a bit undefined if wrong combination of parameters is specified. If table_resources is undefined and schema_fields are defined and source_objects and bucket are, I am not sure what happens here. I think all the "superfluous" combinations where same data is provided by different means should result in exception

Comment thread airflow/providers/google/cloud/operators/bigquery.py Outdated
@turbaszek
turbaszek force-pushed the refactor-bq-operators branch 4 times, most recently from 387991c to 603ea3d Compare May 21, 2020 10:00
@turbaszek
turbaszek force-pushed the refactor-bq-operators branch from 603ea3d to 173563e Compare May 25, 2020 11:14
@turbaszek
turbaszek force-pushed the refactor-bq-operators branch from 173563e to bfedd51 Compare May 25, 2020 12:14
@turbaszek
turbaszek merged commit 3994030 into apache:master May 26, 2020
@turbaszek
turbaszek deleted the refactor-bq-operators branch May 26, 2020 08:03
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants