Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix BigQueryCursor execute method if the location is missing #51

Closed

Conversation

e-galan
Copy link

@e-galan e-galan commented May 9, 2024

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.

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.

Copy link

@ahidalgob ahidalgob left a comment

Choose a reason for hiding this comment

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

Hi, could you elaborate on how you tested this change?

airflow/providers/google/cloud/hooks/bigquery.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/hooks/bigquery.py Outdated Show resolved Hide resolved
@e-galan e-galan force-pushed the fix-bigquery-value-check-operator_job_location_bug branch from c503a8b to 7ad8579 Compare May 9, 2024 09:44
@e-galan
Copy link
Author

e-galan commented May 9, 2024

Hi, could you elaborate on how you tested this change?

@ahidalgob I ran a dag with a BigQueryValueCheckOperator task in a Composer environment that contained the code, with and without locations specified. With this code the BigQueryValueCheckOperator task finished successfully.

Adding new unit tests wasn't particularly useful because the execute() method essentially contained two client method calls (self._run_query(sql) and self._get_query_result()) that can only be mocked and not truly tested.

No existing unit tests have been broken / affected by this fix.

@VladaZakharova VladaZakharova requested a review from ahidalgob May 13, 2024 09:13
@e-galan e-galan closed this May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants