airflow
airflow copied to clipboard
Fix BigQueryCursor execute method if the location is missing
This addresses a bug where BigQueryValueCheckOperator
tasks failed if it was not provided with the location parameter and the dataset location was outside of the US.
If the location parameter is not provided to BigQueryValueCheckOperator
, then the location parameter will still be None when execution reaches the BigQueryCursor
execute method. The execute method makes two client requests: the first to the BigQuery client, which can work without the location provided, the second to the Google Discovery client, which cannot find a resource without the location resulting in a error.
Pulling the location from a successful BigQuery request result resolves that problem.
Change list:
- Make BigQueryCursor use the job location in the case when the location is not provided during the cursor's instantiation.
- Update typing and docstrings.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 newsfragment file, named {pr_number}.significant.rst
or {issue_number}.significant.rst
, in newsfragments.
is it feasible to add the test case for the bug? Can you fix the failing static checks?
@dirrao I can add a unit test, but it will be totally dependent on mocks, because the problem happens between the two client requests. Will that do?
I can't fix the static check, it fails because of smth that does not seem related to my PR:
This static check failure not relevant your PR, it will be fix (at least temporary) by https://github.com/apache/airflow/pull/39662
@dirrao I can add a unit test, but it will be totally dependent on mocks, because the problem happens between the two client requests. Will that do?
We have system tests for google so probably we should modify/add coverage there. @VladaZakharova WDYT?
@eladkal
Yes, thank you , will be good to add system test here to cover this scenario: f.e. try to create dataset and table with location different from US
and then run BigQueryValueCheckOperator without specifying location.
@e-galan Please, add the changes accordingly
I have added the system test @dirrao @eladkal . Please take a look.
Hi @eladkal @potiuk ! I guess we have made all the changes, can you please take a look again? :)