Skip to content

Adds "disable config" sample to manage_transfer_configs.py #13578

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+32 −0
Open

Adds "disable config" sample to manage_transfer_configs.py #13578

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions bigquery-datatransfer/snippets/manage_transfer_configs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,38 @@ def update_credentials_with_service_account(override_values={}):
# Return the config name for testing purposes, so that it can be deleted.
return transfer_config

def disable_config(override_values={}):
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Using a mutable object like a dictionary as a default argument can lead to unexpected behavior if it's ever modified, as the modification will persist across calls. It's safer to use None as the default.1

After applying this suggestion, please add the following lines at the beginning of the function body:

    if override_values is None:
        override_values = {}

This pattern is used throughout the file and should ideally be corrected everywhere for consistency and robustness.

Suggested change
def disable_config(override_values={}):
def disable_config(override_values=None):

Style Guide References

Footnotes

  1. Avoid using mutable default arguments in function definitions. Instead, use a default value of None and initialize the mutable type within the function. This prevents hard-to-debug issues where a default object is modified and that modification persists across function calls. ↩

# [START bigquerydatatransfer_disable_config]
from google.cloud import bigquery_datatransfer
from google.protobuf import field_mask_pb2

transfer_client = bigquery_datatransfer.DataTransferServiceClient()

transfer_config_name = "projects/1234/locations/us/transferConfigs/abcd"
# [END bigquerydatatransfer_disable_config]
# To facilitate testing, we replace values with alternatives
# provided by the testing harness.
transfer_config_name = override_values.get(
"transfer_config_name", transfer_config_name
)
# [START bigquerydatatransfer_disable_config]

transfer_config = bigquery_datatransfer.TransferConfig(name=transfer_config_name)
transfer_config.disabled = True

transfer_config = transfer_client.update_transfer_config(
{
"transfer_config": transfer_config,
"update_mask": field_mask_pb2.FieldMask(paths=["disabled"]),
}
)
Comment on lines +126 to +131
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

For better readability, consider using keyword arguments when calling update_transfer_config instead of passing a dictionary. This makes the call more explicit and easier to understand at a glance.

    transfer_config = transfer_client.update_transfer_config(
        transfer_config=transfer_config,
        update_mask=field_mask_pb2.FieldMask(paths=["disabled"]),
    )


print(f"Updated config: '{transfer_config.name}'")
print(f"Is config disabled: '{transfer_config.disabled}'")
# [END bigquerydatatransfer_disable_config]
# Return the config name for testing purposes, so that it can be deleted.
return transfer_config
Comment on lines +107 to +137
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The newly added disable_config function is missing a corresponding unit test in manage_transfer_configs_test.py. Adding a test is essential to verify the functionality of this sample and prevent future regressions. Please add a test that calls this function and asserts that the transfer config is disabled correctly.



def schedule_backfill_manual_transfer(override_values={}):
# [START bigquerydatatransfer_schedule_backfill]
Expand Down