Add BigTable source format in BigQuery tables #4155

jmthvt · 2020-10-28T10:00:19Z

This is adding the BIGTABLE source format in BigQuery tables.
Fixes hashicorp/terraform-provider-google#7653

If this PR is for Terraform, I acknowledge that I have:

Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

bigquery: Added BigTable source format in BigQuery table

modular-magician · 2020-10-28T10:01:36Z

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review.

Thanks for your contribution! A human will be with you soon.

@slevenick, please review this PR or find an appropriate assignee.

modular-magician · 2020-10-28T10:09:33Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 38 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 38 insertions(+), 5 deletions(-))

modular-magician · 2020-10-28T10:16:37Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 62 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 62 insertions(+), 5 deletions(-))

slevenick · 2020-10-28T16:21:07Z

third_party/terraform/tests/resource_bigquery_table_test.go

+		CheckDestroy: testAccCheckBigQueryTableDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccBigQueryTableFromBigtable(context),


Looks like this doesn't match the function declaration, causing this error:
google-beta/resource_bigquery_table_test.go:432:13: undefined: testAccBigQueryTableFromBigtable

oops, thanks!
Should be fixed now.

modular-magician · 2020-10-29T01:44:41Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 62 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 62 insertions(+), 5 deletions(-))

slevenick · 2020-10-29T15:37:57Z

/gcbrun

modular-magician · 2020-10-29T15:38:11Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=155026"

modular-magician · 2020-10-29T15:46:40Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 62 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 62 insertions(+), 5 deletions(-))

modular-magician · 2020-10-29T17:12:41Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceSpannerInstance_basic|TestAccBigqueryDataTransferConfig|TestAccBigQueryDataTable_bigtable You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=155032"

slevenick · 2020-10-29T17:37:53Z

third_party/terraform/tests/resource_bigquery_table_test.go

+		  ignore_unknown_values = true
+
+		  source_uris = [
+			"https://googleapis.com/bigtable/projects/project_id/instances/instance_id/tables/table_name",


Looks like this is causing an error during testing. This is an invalid source, maybe try using one of the source_uris from another test?

I'll try to create a bigtable in the test & generate the URI.

@slevenick that's added but I'm unable to run the tests - can you please try again?

slevenick · 2020-10-29T17:41:46Z

third_party/terraform/tests/resource_bigquery_table_test.go

@@ -416,6 +416,30 @@ func TestAccBigQueryDataTable_sheet(t *testing.T) {
 	})
 }

+func TestAccBigQueryDataTable_bigtable(t *testing.T) {


This test should only be run for the beta provider, right? This file may need to be renamed to end in .go.erb and wrap this test in if/else tags to check the version.

An example can be seen here: https://github.com/GoogleCloudPlatform/magic-modules/blob/master/third_party/terraform/tests/resource_binaryauthorization_policy_test.go.erb#L41

That's quite a weird one actually:
As per my understanding, you can set the format to BIGTABLE in stable (search for sourceFormat in https://bigquery.googleapis.com/discovery/v1/apis/bigquery/v2/rest )
However only 2 regions are supported for the queries https://cloud.google.com/bigquery/external-data-bigtable#supported_regions_and_zones.

From the BigQuery API point of view, I don't think it requires the beta provider.
But from BigTable it does.
Not sure of the preferred approach here.

modular-magician · 2020-10-30T07:25:04Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 62 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 62 insertions(+), 5 deletions(-))

…gic-modules into feat/add-bigtable-sourceformat

modular-magician · 2020-11-02T07:32:05Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 88 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 88 insertions(+), 5 deletions(-))

venkykuberan · 2020-11-02T23:22:44Z

third_party/terraform/resources/resource_bigquery_table.go

@@ -153,9 +153,9 @@ func resourceBigQueryTable() *schema.Resource {
 						"source_format": {
 							Type:        schema.TypeString,
 							Required:    true,
-							Description: `The data format. Supported values are: "CSV", "GOOGLE_SHEETS", "NEWLINE_DELIMITED_JSON", "AVRO", "PARQUET", and "DATASTORE_BACKUP". To use "GOOGLE_SHEETS" the scopes must include "googleapis.com/auth/drive.readonly".`,
+							Description: `The data format. Supported values are: "CSV", "GOOGLE_SHEETS", "NEWLINE_DELIMITED_JSON", "AVRO", "PARQUET", "DATSTORE_BACKUP", and "BIGTABLE". To use "GOOGLE_SHEETS" the scopes must include "googleapis.com/auth/drive.readonly".`,


I wonder ORC can also included in the source_format list ?. We have an open issue (hashicorp/terraform-provider-google#7691) for that type.

Hi, sorry but that's out of scope for this PR - adding it in the list is trivial but requires to write the tests.

slevenick · 2020-11-03T00:39:39Z

/gcbrun

modular-magician · 2020-11-03T00:39:53Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=155702"

modular-magician · 2020-11-03T00:48:27Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 88 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 88 insertions(+), 5 deletions(-))

jmthvt · 2020-11-10T04:15:26Z

@slevenick are the tests passing?

slevenick · 2020-11-10T18:20:19Z

/gcbrun

modular-magician · 2020-11-10T18:20:35Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=157044"

modular-magician · 2020-11-10T18:29:23Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 88 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 88 insertions(+), 5 deletions(-))

modular-magician · 2020-11-10T19:36:11Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceGameServicesGameServerDeploymentRollout_basic|TestAccDataSourceSpannerInstance_basic|TestAccApiGatewayApiConfigIamMemberGenerated|TestAccApiGatewayApiConfigIamBindingGenerated|TestAccApiGatewayApiConfigIamPolicyGenerated|TestAccApiGatewayGatewayIamBindingGenerated|TestAccApiGatewayGatewayIamMemberGenerated|TestAccApiGatewayGatewayIamPolicyGenerated|TestAccApiGatewayApiConfig_apigatewayApiConfigBasicExample|TestAccApiGatewayApiConfig_apigatewayApiConfigBasicExampleUpdated|TestAccApiGatewayGateway_apigatewayGatewayBasicExample|TestAccApiGatewayGateway_apigatewayGatewayBasicExampleUpdated|TestAccBigqueryDataTransferConfig|TestAccBigQueryDataTable_bigtable|TestAccCloudRunService_cloudRunServiceMultipleEnvironmentVariablesExample|TestAccCloudRunService_cloudRunServiceUpdate|TestAccCloudRunDomainMapping_cloudRunDomainMappingBasicExample|TestAccCloudFunctionsFunction_update|TestAccCloudFunctionsFunction_vpcConnector|TestAccGameServicesGameServerConfig_gameServiceConfigBasicExample|TestAccGameServicesGameServerDeploymentRollout_gameServiceDeploymentRolloutBasicExample You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=157062"

slevenick · 2020-11-10T22:16:57Z

third_party/terraform/tests/resource_bigquery_table_test.go

+		instance_name = google_bigtable_instance.instance.name
+
+		column_family {
+			family = cf-"%{random_suffix}-first"


Looks like this is incorrect. Quotes should wrap the cf-.

Error: Missing newline after argument on config893095785/terraform_plugin_test.tf line 19, in resource "google_bigtable_table" "table": 19: family = cf-"j42ngv1ayb-first" An argument definition must end with a newline.

sorry for that - fixed now.

modular-magician · 2020-11-25T17:48:37Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceSpannerInstance_basic|TestAccApiGatewayApiConfigIamBindingGenerated|TestAccApiGatewayApiConfigIamMemberGenerated|TestAccApiGatewayApiConfigIamPolicyGenerated|TestAccApiGatewayGatewayIamBindingGenerated|TestAccApiGatewayGatewayIamMemberGenerated|TestAccApiGatewayGatewayIamPolicyGenerated|TestAccApiGatewayApiConfig_apigatewayApiConfigBasicExample|TestAccApiGatewayApiConfig_apigatewayApiConfigBasicExampleUpdated|TestAccApiGatewayGateway_apigatewayGatewayBasicExample|TestAccApiGatewayGateway_apigatewayGatewayBasicExampleUpdated|TestAccActiveDirectoryDomainTrust_activeDirectoryDomainTrustBasicExample|TestAccBigQueryDataTable_bigtable|TestAccBigqueryDataTransferConfig|TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample|TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample|TestAccComposerEnvironment_withUpdateOnCreate|TestAccComputeTargetInstance_targetInstanceCustomNetworkExample|TestAccFilestoreInstance_filestoreInstanceBasicExample|TestAccFilestoreInstance_update|TestAccProjectIamCustomRole_basic|TestAccOSLoginSSHPublicKey_osLoginSshKeyExpiry|TestAccSpannerDatabase_basic|TestAccStorageBucket_cors You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=159876"

jmthvt · 2020-12-08T08:56:55Z

@slevenick 👀
Sorry to bother really. If there is a way for me to look at those tests, please let me know!

slevenick · 2020-12-08T17:11:45Z

@slevenick 👀
Sorry to bother really. If there is a way for me to look at those tests, please let me know!

Error: Error creating instance. rpc error: code = InvalidArgument desc = Error in field 'instance_id' : Invalid id for collection instances : Should match [a-z][a-z0-9\-]+[a-z0-9] but found 'tf_test_bigtable_inst_aq1wv0hf8g'

Looks like that the instance_id is in the wrong format. Are you able to compile magic modules? It should produce a provider that you can then run the tests on: https://github.com/GoogleCloudPlatform/magic-modules#testing-your-changes

modular-magician · 2021-03-29T07:43:02Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 4 files changed, 7 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 4 files changed, 7 insertions(+), 5 deletions(-))

modular-magician · 2021-03-29T07:43:22Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=179544"

modular-magician · 2021-03-29T09:28:56Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 512 insertions(+), 1011 deletions(-))
Terraform Beta: Diff ( 5 files changed, 512 insertions(+), 1011 deletions(-))

modular-magician · 2021-03-29T09:29:09Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=179546"

modular-magician · 2021-03-29T10:57:03Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceCloudIdentityGroupMemberships_basic|TestAccDataSourceGameServicesGameServerDeploymentRollout_basic|TestAccBigqueryDataTransferConfig|TestAccBigQueryTable_Basic|TestAccBigQueryTable_Kms|TestAccBigQueryTable_HivePartitioning|TestAccBigQueryTable_HourlyTimePartitioning|TestAccBigQueryTable_HivePartitioningCustomSchema|TestAccBigQueryDataTable_bigtable|TestAccBigQueryTable_RangePartitioning|TestAccBigQueryTable_MonthlyTimePartitioning|TestAccBigQueryTable_YearlyTimePartitioning|TestAccBigQueryTable_View|TestAccBigQueryTable_MaterializedView_DailyTimePartioning_Basic|TestAccBigQueryDataTable_sheet|TestAccBigQueryTable_updateView|TestAccBigQueryExternalDataTable_CSV|TestAccBigQueryTable_MaterializedView_DailyTimePartioning_Update|TestAccCloudRunDomainMapping_cloudRunDomainMappingBasicExample|TestAccCloudRunService_cloudRunServiceMultipleEnvironmentVariablesExample|TestAccCloudFunctionsFunction_vpcConnector|TestAccComputeRegionTargetHttpProxy_regionTargetHttpProxyBasicExample|TestAccComputeTargetInstance_targetInstanceCustomNetworkExample|TestAccNotebooksInstance_notebookInstanceFullExample|TestAccSpannerDatabase_basic You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=179548"

modular-magician · 2021-03-29T11:09:46Z

Tests failed during RECORDING mode: TestAccBigQueryTable_HivePartitioning|TestAccBigQueryTable_HivePartitioningCustomSchema|TestAccBigQueryTable_RangePartitioning|TestAccBigQueryTable_View|TestAccBigQueryTable_HourlyTimePartitioning|TestAccBigQueryTable_Kms|TestAccBigQueryTable_MonthlyTimePartitioning|TestAccBigQueryTable_YearlyTimePartitioning|TestAccBigQueryTable_Basic|TestAccBigQueryDataTable_bigtable|TestAccBigQueryTable_MaterializedView_DailyTimePartioning_Basic|TestAccBigQueryDataTable_sheet|TestAccBigQueryExternalDataTable_CSV|TestAccBigQueryTable_updateView|TestAccBigQueryTable_MaterializedView_DailyTimePartioning_Update|TestAccNotebooksInstance_notebookInstanceFullExample|TestAccCloudFunctionsFunction_vpcConnector Please fix these to complete your PR

slevenick · 2021-03-29T16:03:41Z

It looks like there are some bad merge issues going on here with the tests. The test file shows a lot of changes that I don't think are associated with your PR. Can you fix that before I can review this again?

jmthvt · 2021-03-30T02:21:04Z

Yes I will, I've noticed the bad diff too.
Sorry for letting the PR rot, I'm resuming this, set up a sandbox and let you know once ready.

modular-magician · 2021-03-30T10:55:40Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 78 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 6 files changed, 79 insertions(+), 6 deletions(-))

modular-magician · 2021-03-30T10:56:00Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=179726"

modular-magician · 2021-03-30T12:24:22Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccBigQueryDataTable_bigtable|TestAccCloudFunctionsFunction_vpcConnector|TestAccNotebooksInstance_notebookInstanceFullExample You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=179728"

modular-magician · 2021-03-30T12:48:46Z

Tests failed during RECORDING mode: TestAccBigQueryDataTable_bigtable|TestAccNotebooksInstance_notebookInstanceFullExample Please fix these to complete your PR

jmthvt · 2021-04-15T07:50:17Z

@slevenick Finally tests are passing locally, this is now ready to review.
My apologies for the chaotic PR and thanks a lot for your patience.

modular-magician · 2021-04-15T07:57:38Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 80 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 80 insertions(+), 5 deletions(-))

modular-magician · 2021-04-15T07:57:47Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=182504"

modular-magician · 2021-04-15T09:25:13Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccBigQueryDataTable_bigtable|TestAccCloudRunDomainMapping_foregroundDeletion|TestAccCloudFunctionsFunction_vpcConnector|TestAccComputeRouterPeer_advertiseMode You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=182505"

modular-magician · 2021-04-15T09:41:30Z

Tests failed during RECORDING mode: TestAccCloudRunDomainMapping_foregroundDeletion Please fix these to complete your PR

slevenick

Looks reasonable, thanks!

Add BigTable source format in BigQuery

bf4a252

google-cla bot added the cla: yes label Oct 28, 2020

modular-magician requested a review from slevenick October 28, 2020 10:01

Add tests

828eb0a

slevenick requested changes Oct 28, 2020

View reviewed changes

Fix test config typos

f503762

slevenick requested changes Oct 29, 2020

View reviewed changes

Merge branch 'master' into feat/add-bigtable-sourceformat

4e6bf18

jmthvt added 2 commits November 2, 2020 15:22

Add bigtable external source in the bigquery tests

0fdd96b

Merge branch 'feat/add-bigtable-sourceformat' of github.com:jmthvt/ma…

137dff6

…gic-modules into feat/add-bigtable-sourceformat

venkykuberan reviewed Nov 2, 2020

View reviewed changes

slevenick requested changes Nov 10, 2020

View reviewed changes

Merge master & resolve test conflict

e992d4e

jmthvt force-pushed the feat/add-bigtable-sourceformat branch from 2222352 to e992d4e Compare March 30, 2021 10:46

jmthvt added 4 commits April 12, 2021 16:17

Merge branch 'master' into feat/add-bigtable-sourceformat

7c848b2

Merge branch 'master' into feat/add-bigtable-sourceformat

7eb6f1d

Merge branch 'master' into feat/add-bigtable-sourceformat

194aeb7

Ignore deletion_protection in TestAccBigQueryDataTable_bigtable

2973a68

slevenick approved these changes Apr 15, 2021

View reviewed changes

slevenick merged commit e7ef590 into GoogleCloudPlatform:master Apr 15, 2021

This was referenced Apr 15, 2021

Add BigTable source format in BigQuery tables hashicorp/terraform-provider-google#8923

Merged

Add BigTable source format in BigQuery tables hashicorp/terraform-provider-google-beta#3165

Merged

Add BigTable source format in BigQuery tables #4155

Add BigTable source format in BigQuery tables #4155

Conversation

jmthvt commented Oct 28, 2020 • edited Loading

modular-magician commented Oct 28, 2020

modular-magician commented Oct 28, 2020

Diff report:

modular-magician commented Oct 28, 2020

Diff report:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Oct 29, 2020

Diff report:

slevenick commented Oct 29, 2020

modular-magician commented Oct 29, 2020

modular-magician commented Oct 29, 2020

Diff report:

modular-magician commented Oct 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmthvt Oct 30, 2020 • edited Loading

Choose a reason for hiding this comment

modular-magician commented Oct 30, 2020

Diff report:

modular-magician commented Nov 2, 2020

Diff report:

Choose a reason for hiding this comment

jmthvt Nov 10, 2020 • edited Loading

Choose a reason for hiding this comment

slevenick commented Nov 3, 2020

modular-magician commented Nov 3, 2020

modular-magician commented Nov 3, 2020

Diff report:

jmthvt commented Nov 10, 2020

slevenick commented Nov 10, 2020

modular-magician commented Nov 10, 2020

modular-magician commented Nov 10, 2020

Diff report:

modular-magician commented Nov 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Nov 25, 2020

jmthvt commented Dec 8, 2020

slevenick commented Dec 8, 2020

modular-magician commented Mar 29, 2021

Diff report:

modular-magician commented Mar 29, 2021

modular-magician commented Mar 29, 2021

Diff report:

modular-magician commented Mar 29, 2021

modular-magician commented Mar 29, 2021

modular-magician commented Mar 29, 2021

slevenick commented Mar 29, 2021

jmthvt commented Mar 30, 2021

modular-magician commented Mar 30, 2021

Diff report:

modular-magician commented Mar 30, 2021

modular-magician commented Mar 30, 2021

modular-magician commented Mar 30, 2021

jmthvt commented Apr 15, 2021

modular-magician commented Apr 15, 2021

Diff report:

modular-magician commented Apr 15, 2021

modular-magician commented Apr 15, 2021

modular-magician commented Apr 15, 2021

slevenick left a comment

Choose a reason for hiding this comment

jmthvt commented Oct 28, 2020 •

edited

Loading

jmthvt Oct 30, 2020 •

edited

Loading

jmthvt Nov 10, 2020 •

edited

Loading