Skip to content
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

Terraform BigQuery Table Hive partitioning support #3335

Merged

Conversation

ffung
Copy link
Contributor

@ffung ffung commented Apr 3, 2020

Fixes: hashicorp/terraform-provider-google#5664
As of March 2, range partioning / hive partitioning is GA, see https://cloud.google.com/bigquery/docs/release-notes.
Note: Doesn't support require_partition_filter attribute as this isn't available from the used BigQuery SDK.

Release Note Template for Downstream PRs (will be copied)

bigquery: Added support for `google_bigquery_table` `hive_partitioning_options`
bigquery: Added `google_bigquery_table` `range_partitioning` to GA

@modular-magician
Copy link
Collaborator

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
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 3 files changed, 318 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 3 files changed, 152 insertions(+), 5 deletions(-))

@modular-magician
Copy link
Collaborator

Oops! It looks like you're using an unknown release-note type in your changelog entries:

  • REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

@ffung
Copy link
Contributor Author

ffung commented Apr 3, 2020

± env |grep GOOG
GOOGLE_REGION=us-central1
GOOGLE_ZONE=us-central1-a

± make testacc TEST=./google TESTARGS='-run=TestAccBigQueryTable_Hive'

==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccBigQueryTable_Hive -timeout 240m -ldflags="-X=github.com/terraform-providers/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccBigQueryTable_HivePartitioning
=== PAUSE TestAccBigQueryTable_HivePartitioning
=== CONT  TestAccBigQueryTable_HivePartitioning
--- PASS: TestAccBigQueryTable_HivePartitioning (8.96s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	(cached)

@danawillow danawillow requested a review from slevenick April 14, 2020 23:43
Copy link
Contributor

@slevenick slevenick left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Sorry it took so long to review, but this all looks great. Just a few notes on docs/tests. When you push a new commit it should successfully complete the checks as I standardized the release note comment

@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 3 files changed, 321 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 3 files changed, 155 insertions(+), 5 deletions(-))

1 similar comment
@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 3 files changed, 321 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 3 files changed, 155 insertions(+), 5 deletions(-))

Copy link
Contributor

@slevenick slevenick left a comment

Choose a reason for hiding this comment

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

Minor issues and this will likely require a rebase due to an unrelated PR that changed test structure.

If you can do the rebase I'll get this merged in, looks good!

@ffung ffung force-pushed the tf_bigquery_hive_partitioning_5664 branch from 0db16d3 to da02a5e Compare April 17, 2020 08:13
@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 3 files changed, 321 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 3 files changed, 155 insertions(+), 5 deletions(-))

1 similar comment
@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 3 files changed, 321 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 3 files changed, 155 insertions(+), 5 deletions(-))

@ffung ffung force-pushed the tf_bigquery_hive_partitioning_5664 branch from de0518e to cb238ce Compare May 6, 2020 14:31
@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 3 files changed, 327 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 161 insertions(+), 9 deletions(-))

@ffung ffung requested a review from slevenick May 6, 2020 14:38
@ffung
Copy link
Contributor Author

ffung commented May 20, 2020

@slevenick I think I resolved all the issues, is there anything I still can do for this PR?

@slevenick
Copy link
Contributor

@slevenick I think I resolved all the issues, is there anything I still can do for this PR?

Ah, sorry lost track of this. Looks like there are conflicts now with master, could you rebase on master and I can run tests and get this merged?

@ffung ffung force-pushed the tf_bigquery_hive_partitioning_5664 branch from cb238ce to cdfef07 Compare May 21, 2020 08:16
@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 3 files changed, 327 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 161 insertions(+), 9 deletions(-))

@ffung
Copy link
Contributor Author

ffung commented May 21, 2020

@slevenick done.I cannot see why the build fails, but let me know if I can help address the issue.

@slevenick
Copy link
Contributor

@slevenick done.I cannot see why the build fails, but let me know if I can help address the issue.

Looks like the files aren't formatted correctly:

gofmt needs running on the following files:
./google/resource_bigquery_table_test.go
./google/resource_bigquery_table.go```

You can generate the downstream files, run `gofmt` on them and then copy them back into Magic Modules to fix this

@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 3 files changed, 321 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 3 files changed, 155 insertions(+), 5 deletions(-))

@ffung
Copy link
Contributor Author

ffung commented May 21, 2020

@slevenick ran gofmt, build still fails though

@slevenick
Copy link
Contributor

@slevenick ran gofmt, build still fails though

Looks like the problem is now:

google/resource_bigquery_table_test.go:69:30: not enough arguments in call to testBucketName
	have ()
	want (*testing.T)

You can run make test in the generated provider to make sure everything passes

@ffung ffung force-pushed the tf_bigquery_hive_partitioning_5664 branch from 75807a4 to c101365 Compare May 22, 2020 19:59
@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 3 files changed, 321 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 3 files changed, 155 insertions(+), 5 deletions(-))

@ffung
Copy link
Contributor Author

ffung commented May 22, 2020

@slevenick ah sorry that was sloppy, tests are running succesfully now locally, but the builds still seems to fail.

@slevenick
Copy link
Contributor

Sorry the process is a little rough around the edges. Looks like we enforce that gofmt must be run with a specific flag 😞

Step #22 - "tpgb-test": google-beta/resource_bigquery_table.go:336: File is not `gofmt`-ed with `-s` (gofmt)
Step #22 - "tpgb-test": 			"range_partitioning": &schema.Schema{```

@ffung ffung force-pushed the tf_bigquery_hive_partitioning_5664 branch from c101365 to 5d55d27 Compare May 27, 2020 12:18
@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 3 files changed, 320 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 3 files changed, 152 insertions(+), 2 deletions(-))

@ffung
Copy link
Contributor Author

ffung commented May 27, 2020

@slevenick that should fix it;-)

Copy link
Contributor

@slevenick slevenick left a comment

Choose a reason for hiding this comment

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

Thanks for the addition!

@LoekL
Copy link

LoekL commented Jun 24, 2020

FYI I tried using this today, no dice. The table does get created:

Screenshot 2020-06-23 18 11 31

But there's no sign of the partitions in the table schema:

Screenshot 2020-06-23 18 12 31

If I however run:

bq mkdef \
  --autodetect \
  --ignore_unknown_values \
  --source_format=NEWLINE_DELIMITED_JSON \
  --hive_partitioning_mode=CUSTOM \
  --hive_partitioning_source_uri_prefix=gs://anhistous-metonymic-7578834-analytics-staging-batched/data_type=jsonl/event_schema=playfab/{event_category:STRING}/{event_environment:STRING}/{event_date:DATE}/{event_hour:STRING}/{event_minute:STRING} \
  gs://anhistous-metonymic-7578834-analytics-staging-batched/data_type=jsonl/event_schema=playfab/\* \
  AnalyticsEnvironment:STRING,PlayFabEnvironment:STRING,SourceType:STRING,Source:STRING,EventNamespace:STRING,TitleId:STRING,GroupBatchId:STRING,BatchId:STRING,EventId:STRING,EventName:STRING,EntityType:STRING,EntityId:STRING,Timestamp:TIMESTAMP,ReceivedTimestamp:TIMESTAMP,BatchedTimestamp:TIMESTAMP,BatchJobName:STRING,EventAttributes:STRING \
  > /Users/loek/Desktop/events_playfab_staging_hive_partitioned_batched

Followed by:

bq mk --table --location=US --external_table_definition=/Users/loek/Desktop/events_playfab_staging_hive_partitioned_batched external.events_playfab_hive_partitioned_batched

It does work:

Screenshot 2020-06-23 18 15 54

&

Screenshot 2020-06-23 18 16 00

The only difference within the BQ UI I can see is that when going via the bq CLI, I don't see Compression = GZIP in the table details. Am using Terraform v0.12.18 & provider.google v3.26.0.

@slevenick
Copy link
Contributor

@LoekL can you create a new issue on https://github.com/terraform-providers/terraform-provider-google with steps to reproduce this problem so that we can look into this?

@ffung
Copy link
Contributor Author

ffung commented Jun 25, 2020

yes @LoekL please raise an issue, I cannot reproduce your problem. If I apply this, hive partitioning works just fine.

resource "google_bigquery_dataset" "test" {
  dataset_id                  = "foo"
  friendly_name               = "test"
  description                 = "This is a test description"
  location                    = "europe-west3"
  default_table_expiration_ms = 3600000

  labels = {
    env = "default"
  }
}

resource "google_bigquery_table" "test" {
  dataset_id = google_bigquery_dataset.test.dataset_id
  table_id   = "bar"

  external_data_configuration {
    source_format = "PARQUET"
    autodetect = true
    source_uris= ["gs://project-2resdfdsafsdf43/test/*"]

    hive_partitioning_options {
      mode = "CUSTOM"
      source_uri_prefix = "gs://project-2resdfdsafsdf43/test/{part:STRING}/{dt:DATE}"
    }
  }
}

1
2

@LoekL
Copy link

LoekL commented Jun 25, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BigQuery external tables should be able to be defined using HivePartitioningOptions
5 participants