danawillow
commented
4 years ago The merged PR fixes most of these. Here's a list of resources which, according to their documentation, don't support timeouts (grep -rL 'Timeouts' website/docs/r/):
- bigquery_table
- ~bigtable_gc_policy~ https://github.com/hashicorp/terraform-provider-google/issues/13045
- ~bigtable_instance~
- ~bigtable_table~
- cloudiot_registry
- compute_security_policy
- container_registry
- dataflow_job
- dns_record_set
- folder
- folder_organization_policy
- logging_billing_account_exclusion
- logging_billing_account_sink
- logging_folder_exclusion
- logging_folder_sink
- logging_organization_exclusion
- logging_organization_sink
- logging_project_exclusion
- logging_project_sink
- organization_policy
- project
- project_organization_policy
- project_service
- runtimeconfig_config
- runtimeconfig_variable
- service_account
- service_account_key
- service_networking_connection
- storage_bucket
- storage_bucket_acl
- storage_bucket_object
- storage_default_object_acl
- storage_notification
- storage_object_acl
- storage_transfer_job
I didn't bother checking whether the docs+code matched. Also, no IAM resources support timeouts as far as I'm aware. Before doing IAM ones, we'd probably want to convert the eventual consistency checks into the framework introduced in https://github.com/terraform-providers/terraform-provider-google/issues/4993.
For resources with synchronous calls, we could add retry logic which takes a timeout. For extra credit, we could send a context with a timeout into the request that we send, though I think our synchronous calls tend to return quickly, so I don't think it would really be worth it.
grayside
And we should get rid of the OperationWait() methods that don't have them so that we can guarantee across the codebase that we're using them.
Inspired by https://github.com/GoogleCloudPlatform/magic-modules/pull/2001, where a user caught a bug where one of our resources had a timeouts block declared, but it wasn't used everywhere, and so their timeouts didn't work. In the past, we could make the case that the user should check the docs to see which resources do and don't support timeouts, but it's harder to make now that we have so many resources that do, and so few that don't.