-
Notifications
You must be signed in to change notification settings - Fork 1.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
Additional bigquery locations #2566
Additional bigquery locations #2566
Conversation
Looks like two of these were already added in #2333. Can you update your PR to be against the 2.0.0 branch and fix any merge conflicts that arise as a result of that? Thanks! |
1d72845
to
96e94a4
Compare
Thanks @danawillow, I have rebased to 2.0.0. |
There are now even more locations available for BigQuery datasets: |
CheckDestroy: testAccCheckBigQueryDatasetDestroy, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: testAccBigQueryRegionalDataset(datasetID1, "asia-east1"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI we don't tend to test all our resources in every region they could be created in (at some point we have to assume that the API works as documented). Is there a reason we should treat bigquery datasets differently, or was this added just to be thorough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just being thorough and as a way to test the validation.
Three more regional locations are supported by BigQuery however the current validation rejects them.
$ make testacc TEST=./google TESTARGS='-run=TestAccBigQueryDataset' ==> Checking that code complies with gofmt requirements... TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccBigQueryDataset -timeout 120m -ldflags="-X=github.com/terraform-providers/terraform-provider-google/version.ProviderVersion=acc" === RUN TestAccBigQueryDataset_basic === RUN TestAccBigQueryDataset_access === RUN TestAccBigQueryDataset_regionalLocation --- PASS: TestAccBigQueryDataset_basic (7.77s) --- PASS: TestAccBigQueryDataset_access (13.74s) --- PASS: TestAccBigQueryDataset_regionalLocation (93.68s) PASS ok github.com/terraform-providers/terraform-provider-google/google 93.731s
bf590f4
to
2cb0119
Compare
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
The resource validation of
location
is restricted to three known locations however there are three more regional locations that can be used. This PR expands the validation to include these and adds a new test to explicitly test the creation in the regional locations. The documentation has also been updated to list the three additional locations.