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

Add project_id as a templated variable in two BQ operators #24768

Merged
merged 3 commits into from
Jul 13, 2022

Conversation

leahecole
Copy link
Contributor

Hi all! This is related to #23129. In BigQuery, fully qualified dataset names can be made using the GCP project ID even if the project ID isn't needed for the underlying API to work. It's not unusual to make project_id a default arg and to have it be templated, especially if you're using many BQ operators together. I've added it to two operators where I have used it to construct a dataset name.

I'm not 100% sure how we test templated fields - I looked at the BQ operator test and didn't see any kind of validation, and the test values we're passing to the mocked calls are not templated. Any recommendations for how to handle that are appreciated!

@boring-cyborg boring-cyborg bot added area:providers provider:google Google (including GCP) related issues labels Jun 30, 2022
@leahecole leahecole requested a review from eladkal June 30, 2022 20:09
@eladkal
Copy link
Contributor

eladkal commented Jun 30, 2022

I'm not 100% sure how we test templated fields - I looked at the BQ operator test and didn't see any kind of validation, and the test values we're passing to the mocked calls are not templated. Any recommendations for how to handle that are appreciated!

You can check this example:

def test_templates(self, _, create_task_instance_of_operator):
dag_id = 'TestS3ToGoogleCloudStorageTransferOperator_test_templates'
ti = create_task_instance_of_operator(
CloudDataTransferServiceS3ToGCSOperator,
dag_id=dag_id,
s3_bucket='{{ dag.dag_id }}',
gcs_bucket='{{ dag.dag_id }}',
description='{{ dag.dag_id }}',
object_conditions={'exclude_prefixes': ['{{ dag.dag_id }}']},
gcp_conn_id='{{ dag.dag_id }}',
task_id=TASK_ID,
)
ti.render_templates()
assert dag_id == ti.task.s3_bucket
assert dag_id == ti.task.gcs_bucket
assert dag_id == ti.task.description
assert dag_id == ti.task.object_conditions['exclude_prefixes'][0]
assert dag_id == ti.task.gcp_conn_id

@eladkal
Copy link
Contributor

eladkal commented Jul 1, 2022

@leahecole I'm fine with merging this without the test (most operators don't have it)

can you please fix the static tests?

@leahecole
Copy link
Contributor Author

Yup - sorry, was OOO for a US holiday, I'll look into it in the next couple of days

@potiuk potiuk merged commit 77626b7 into apache:main Jul 13, 2022
@leahecole
Copy link
Contributor Author

ah - you beat me to it, sorry for slowness! been focused on my intern. thank you @potiuk and @eladkal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants