-
Notifications
You must be signed in to change notification settings - Fork 49
feat: option to use bq connection without check #460
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
Changes from 1 commit
bb77dd6
8544bd1
f066f5f
de5e6fa
8ea4190
3da2f6b
e0c090c
84037d5
6fcb6b7
467eb03
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,9 +126,8 @@ def __init__( | |
bq_location, | ||
bq_dataset, | ||
bq_client, | ||
bq_connection_client, | ||
bq_connection_id, | ||
cloud_resource_manager_client, | ||
bq_connection_manager, | ||
cloud_function_service_account, | ||
cloud_function_kms_key_name, | ||
cloud_function_docker_repository, | ||
|
@@ -140,9 +139,7 @@ def __init__( | |
self._bq_dataset = bq_dataset | ||
self._bq_client = bq_client | ||
self._bq_connection_id = bq_connection_id | ||
self._bq_connection_manager = clients.BqConnectionManager( | ||
bq_connection_client, cloud_resource_manager_client | ||
) | ||
self._bq_connection_manager = bq_connection_manager | ||
self._cloud_function_service_account = cloud_function_service_account | ||
self._cloud_function_kms_key_name = cloud_function_kms_key_name | ||
self._cloud_function_docker_repository = cloud_function_docker_repository | ||
|
@@ -152,12 +149,13 @@ def create_bq_remote_function( | |
): | ||
"""Create a BigQuery remote function given the artifacts of a user defined | ||
function and the http endpoint of a corresponding cloud function.""" | ||
self._bq_connection_manager.create_bq_connection( | ||
self._gcp_project_id, | ||
self._bq_location, | ||
self._bq_connection_id, | ||
"run.invoker", | ||
) | ||
if self._bq_connection_manager: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would think of creating a dummy ConnectionManager class, and when skip_bq_connection_check is True, use the dummy implementation instead of a real one. Then the same logic can apply to all the places, and the user (of the connection_manager) doesn't need to think about the implementations. Well just a suggestion, up to you. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Um, gave it a thought, feels weird to implement a class with a method create_bq_connection which doesn't create anything. I'd prefer to not do that. Let me know if you have strong opinion about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case we'd rename the create_bq_connection to sth like prepare_bq_connection. No need to put too much thoughts on it. Just a suggestion. |
||
self._bq_connection_manager.create_bq_connection( | ||
self._gcp_project_id, | ||
self._bq_location, | ||
self._bq_connection_id, | ||
"run.invoker", | ||
) | ||
|
||
# Create BQ function | ||
# https://cloud.google.com/bigquery/docs/reference/standard-sql/remote-functions#create_a_remote_function_2 | ||
|
@@ -816,6 +814,17 @@ def remote_function( | |
" For more details see https://cloud.google.com/functions/docs/securing/cmek#before_you_begin" | ||
) | ||
|
||
skip_bq_connection_check = ( | ||
False if session is None else session._skip_bq_connection_check | ||
) | ||
bq_connection_manager = ( | ||
None | ||
if skip_bq_connection_check | ||
else clients.BqConnectionManager( | ||
bigquery_connection_client, resource_manager_client | ||
) | ||
) | ||
|
||
def wrapper(f): | ||
if not callable(f): | ||
raise TypeError("f must be callable, got {}".format(f)) | ||
|
@@ -832,9 +841,8 @@ def wrapper(f): | |
bq_location, | ||
dataset_ref.dataset_id, | ||
bigquery_client, | ||
bigquery_connection_client, | ||
bq_connection_id, | ||
resource_manager_client, | ||
bq_connection_manager, | ||
cloud_function_service_account, | ||
cloud_function_kms_key_name, | ||
cloud_function_docker_repository, | ||
|
Uh oh!
There was an error while loading. Please reload this page.