-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
Add support for query parameters to BigQueryCheckOperator (#40556) #40558
Add support for query parameters to BigQueryCheckOperator (#40556) #40558
Conversation
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 Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
Remove unnecessary space
ad2ad3a
to
14d6469
Compare
A unit test is needed for that one. |
That was fast! Thanks for the speedy response. I added a unit test (and added a missing |
You were just lucky and the change is small. Both help. |
Still static checks failing (I recommend to install pre-commit - it will fix things for you) |
8a3f865
to
ee9e2d5
Compare
I fixed the static check. The MySQL failure is a bit more surprising - flaky integration test / separate issue already existing in |
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
Yep. Was flaky it seems |
…) (apache#40558) * Add support for query parameters to BigQueryCheckOperator (apache#40556) Remove unnecessary space * Add a unit test for BigQueryCheckOperator query params; fix missing 'self' reference * Fix lint (apache#40558) --------- Co-authored-by: Alden S. Page <alden.page@doubleverify.com>
BigQueryCheckOperator doesn't support query parameters. Query parameters should always be supported to discourage string-formatting, which can introduce SQL injection vulnerabilities.
The attack surface of BigQueryCheckOperator is basically non-existent. However, using query parameters everywhere is a matter of common sense and good housekeeping. A user of the module shouldn't have to choose query parameters for one operator and string formatting for the other; we should always support the safe route, which is through query parameters.
To add support, we'll imitate BigQueryExecuteQueryOperator's approach of adding a
query_params
argument to the constructor of BigQueryCheckOperator. An alternative approach would have been to make it possible to pass inconfiguration
as BigQueryInsertJobOperator does, but that would have other breaking changes (like making thesql
argument redundant).Closes: #40556