Fix error when create external table using table resource

[tnyz]

affected operator: class BigQueryCreateExternalTableOperator(BaseOperator):
root cause: destination_project_dataset_table is being used in l1201 but cannot pass init check at 1154 where ValueError(“You provided both table_resource and exclusive keywords arguments.“) is returned

---

^ 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 UPDATING.md.

[boring-cyborg]

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 (flake8, mypy and type annotations). Our pre-commits 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 Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment 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 for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: [email protected]
  Slack: https://s.apache.org/airflow-slack

[potiuk]

This is wrong. You should pass desination table via tableReference dict param of "table_resource" : https://cloud.google.com/bigquery/docs/reference/rest/v2/tables

[tnyz]

@potiuk thanks for the feedback, however I am still seeing this issue as line 1157 only supports initializing self.destimation_dataset_table from parameter not from table resource. Do you mind elaborate more and check if the configuration is unreachable from table resource, thanks

[potiuk]

Please read the docs. The new "table_resource" when provided, replaces the usage of many parameters and they are ignored if specified. Whenever you use "table_resource" all provided parameters should be passed through "table_resource" rather than through individual parameters. In this particular case the right way of passing destination_dataset_table is via "table_resource"'s "tableReference" object (https://cloud.google.com/bigquery/docs/reference/rest/v2/TableReference) - where you have to specify project_id, datasetId, table_id (previously this was all passed by single "destination_project_dataset_table" string which was
"(project.)dataset.table'".

If you pass both "table_resource" and "destination_project_dataset_table" and "table_resource", the one passed by "destination_project_dataset_table" will be ignored by the new version of bigquery library - that's why when you use "table_resource" and "destination_project_dataset_table" and "table_resource" together - you get the error.

I agree that this is in super clear from the error mesage, I found it out myself couple of days ago when I corrected some warning messages, I think about improving this a bit because it also jumped for me as rather cryptic communication.

[bmbejcek]

I am also currently facing this issue. If we don't have a destination_project_dataset_table variable, it says that it is missing even when passing in this information through the table_resource schema you mentioned @potiuk. If we include both versions, then it says we can't have duplicate information. Seems like a catch-22.

[potiuk]

There were some default values in the past google provider that was fixed in 5.1.0. The default values were set for few parameters (where they should not be if "table_resource" was specified).

You can set those parameters to None (for example destination_project_dataset_table) when you use table_resource if you use earlier version of the provider.

The new version of the provider is free of that problem I think,

[tnyz]

@potiuk to be more specific, this is the block when executing the command from table resource, using None will result in TableReference.from_string(None) right?

        if self.table_resource:
            tab_ref = TableReference.from_string(self.destination_project_dataset_table)
            bq_hook.create_empty_table(
                table_resource=self.table_resource,
                project_id=tab_ref.project,
                table_id=tab_ref.table_id,
                dataset_id=tab_ref.dataset_id,
            )

[potiuk]

Ah I see now. I have not looked that far.I am not sure though if that is the right fix, I think - looking at the code deeper, is that this is really a mixture of old and new way of passing table resource parameters - there is ambiguity here - looks like the idea was to put everything in table resource and here we have a weird expactation that part of it will be passed as destination_project_dataset_table.

So IMHO the right approach is to change the lines you marked to:

        if self.table_resource:
            bq_hook.create_empty_table(
                table_resource=self.table_resource,
                project_id=None,
                table_id=None,
                dataset_id=None,
            )

[tnyz]

@potiuk I agree, please review the updated pr

[tnyz]

I have not closely examine other operators but they might suffer from the same issue too

[potiuk]

I believe this should be restored now

[tnyz]

good call

[turbaszek]

Does removing this logic break backward compatibility?

[tnyz]

Considering it was incorrectly introduced and table creation via table resource is not properly supported, I don't think this will break backwards compatibility

[potiuk]

Agree.

[github-actions]

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

[potiuk]

Static checks are failing

[tnyz]

Static checks are failing

test has been fixed

[boring-cyborg]

Awesome work, congrats on your first merged pull request!