Glue Crawler: Add catalog target config #9430

azclub · 2019-07-21T10:15:46Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

resource/aws_glue_crawler: Add `catalog_target` attribute

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSGlueCrawler_CatalogTarget'

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSGlueCrawler_CatalogTarget -timeout 120m
=== RUN   TestAccAWSGlueCrawler_CatalogTarget
=== PAUSE TestAccAWSGlueCrawler_CatalogTarget
=== RUN   TestAccAWSGlueCrawler_CatalogTarget_Multiple
=== PAUSE TestAccAWSGlueCrawler_CatalogTarget_Multiple
=== CONT  TestAccAWSGlueCrawler_CatalogTarget
=== CONT  TestAccAWSGlueCrawler_CatalogTarget_Multiple
--- PASS: TestAccAWSGlueCrawler_CatalogTarget (126.62s)
--- PASS: TestAccAWSGlueCrawler_CatalogTarget_Multiple (174.49s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	174.556s

azclub · 2019-07-31T07:33:35Z

Hi @bflad, could you review this, please?

bflad

Hi @azclub 👋 Thanks so much for contributing this. Its off to a great start. A few little testing things and we should be able to get this in. Please reach out if you have any questions or do not have time to implement the feedback. 😄

bflad · 2019-07-31T11:30:58Z

aws/resource_aws_glue_crawler_test.go

-EOF
+
+data "aws_iam_policy" "AWSGlueServiceRole" {
+  arn = "arn:aws:iam::aws:policy/service-role/AWSGlueServiceRole"


Please keep the usage of the aws_partition data source -- we run this acceptance testing across multiple AWS partitions like GovCloud (US).

Suggested change

arn = "arn:aws:iam::aws:policy/service-role/AWSGlueServiceRole"

arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AWSGlueServiceRole"

bflad · 2019-07-31T11:34:10Z

aws/resource_aws_glue_crawler_test.go

@@ -1391,6 +1516,146 @@ resource "aws_glue_crawler" "test" {
 `, rName, rName, path1, path2)
 }

+func testAccGlueCrawlerConfig_CatalogTarget_TableResources(tables []string) string {


Rather than creating a separate function that generates configuration, this can be accomplished within configurations that potentially need multiple tables by using count and count.index with the resource. e.g.

resource "aws_glue_catalog_table" "test" { count = %d database_name = "${aws_glue_catalog_database.test.name}" name = "%s${count.index}" table_type = "EXTERNAL_TABLE" storage_descriptor { location = "s3://${aws_s3_bucket.default.bucket}" } }

The count would be the number to create and the table name would be rName. References can be simply:

aws_glue_catalog_table.test[*].name

bflad · 2019-07-31T11:34:44Z

aws/resource_aws_glue_crawler_test.go

+	return config
+}
+
+func testAccGlueCrawlerConfig_CatalogTarget_TableList(tables []string) string {


Using the noted reference above (aws_glue_catalog_table.test[*].name), this function is extraneous. 👍

bflad · 2019-07-31T11:36:37Z

aws/resource_aws_glue_crawler_test.go

+}
+EOF
+}
+`, rName, rName, rName, rName)


Nit: Go's fmt verbs support indexing, e.g. %[1]q so repeated arguments are not necessary. 👍

azclub · 2019-08-01T09:52:50Z

Hi @bflad, thank you very much for the review! I've updated the PR, it's ready for review again.

The CI failed on make lint and make test, but those commands didn't throw the same error in local. Do you know how to fix it?

Output from acceptance testing:

TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSGlueCrawler_CatalogTarget -timeout 120m
=== RUN   TestAccAWSGlueCrawler_CatalogTarget
=== PAUSE TestAccAWSGlueCrawler_CatalogTarget
=== RUN   TestAccAWSGlueCrawler_CatalogTarget_Multiple
=== PAUSE TestAccAWSGlueCrawler_CatalogTarget_Multiple
=== CONT  TestAccAWSGlueCrawler_CatalogTarget
=== CONT  TestAccAWSGlueCrawler_CatalogTarget_Multiple
--- PASS: TestAccAWSGlueCrawler_CatalogTarget (100.13s)
--- PASS: TestAccAWSGlueCrawler_CatalogTarget_Multiple (146.67s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	146.720s

bflad

Looks great, @azclub, thank you! 🚀

--- PASS: TestAccAWSGlueCrawler_Role_ARN_NoPath (39.80s)
--- PASS: TestAccAWSGlueCrawler_recreates (45.98s)
--- PASS: TestAccAWSGlueCrawler_SchemaChangePolicy (57.52s)
--- PASS: TestAccAWSGlueCrawler_Configuration (59.91s)
--- PASS: TestAccAWSGlueCrawler_SecurityConfiguration (62.03s)
--- PASS: TestAccAWSGlueCrawler_TablePrefix (61.69s)
--- PASS: TestAccAWSGlueCrawler_JdbcTarget_Exclusions (62.45s)
--- PASS: TestAccAWSGlueCrawler_CatalogTarget_Multiple (69.91s)
--- PASS: TestAccAWSGlueCrawler_JdbcTarget_Multiple (70.66s)
--- PASS: TestAccAWSGlueCrawler_Classifiers (71.49s)
--- PASS: TestAccAWSGlueCrawler_Role_Name_Path (76.75s)
--- PASS: TestAccAWSGlueCrawler_Description (79.83s)
--- PASS: TestAccAWSGlueCrawler_S3Target_Multiple (80.29s)
--- PASS: TestAccAWSGlueCrawler_Role_ARN_Path (81.78s)
--- PASS: TestAccAWSGlueCrawler_CatalogTarget (84.14s)
--- PASS: TestAccAWSGlueCrawler_Schedule (86.09s)
--- PASS: TestAccAWSGlueCrawler_S3Target_Exclusions (91.98s)
--- PASS: TestAccAWSGlueCrawler_JdbcTarget (91.91s)
--- PASS: TestAccAWSGlueCrawler_S3Target (124.60s)
--- PASS: TestAccAWSGlueCrawler_DynamodbTarget (124.71s)

ghost · 2019-08-07T17:37:08Z

This has been released in version 2.23.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost · 2019-11-02T14:54:41Z

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. Thanks!

add catalog target config

792bd12

azclub requested a review from a team July 21, 2019 10:15

ghost added size/S Managed by automation to categorize the size of a PR. service/glue Issues and PRs that pertain to the glue service. labels Jul 21, 2019

update crawler read for catalog targets

1f9dc0f

ghost added size/M Managed by automation to categorize the size of a PR. and removed size/S Managed by automation to categorize the size of a PR. labels Jul 21, 2019

add acceptant test for single catalog target.

5f29fa4

ghost added size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. and removed size/M Managed by automation to categorize the size of a PR. labels Jul 21, 2019

add acceptant test for multiple catalog targets

3a316d7

ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Jul 21, 2019

update doc

8187923

ghost added the documentation Introduces or discusses updates to documentation. label Jul 21, 2019

azclub changed the title ~~[WIP] Glue Crawler: Add catalog target config~~ Glue Crawler: Add catalog target config Jul 21, 2019

bflad suggested changes Jul 31, 2019

View reviewed changes

bflad self-assigned this Jul 31, 2019

bflad added the enhancement Requests to existing resources that expand the functionality or scope. label Jul 31, 2019

Helen Fung added 3 commits July 31, 2019 16:12

keep aws_partition data source

e39f0d0

Use param index for string format

6ba2dc8

cleaner test

4c4a338

bflad self-requested a review August 1, 2019 14:25

bflad added this to the v2.23.0 milestone Aug 1, 2019

bflad approved these changes Aug 1, 2019

View reviewed changes

bflad merged commit 0adad4a into hashicorp:master Aug 1, 2019

bflad added a commit that referenced this pull request Aug 1, 2019

Update CHANGELOG for #9430

851304e

ghost locked and limited conversation to collaborators Nov 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Glue Crawler: Add catalog target config #9430

Glue Crawler: Add catalog target config #9430

azclub commented Jul 21, 2019 •

edited

Loading

azclub commented Jul 31, 2019

bflad left a comment

bflad Jul 31, 2019

bflad Jul 31, 2019

bflad Jul 31, 2019

bflad Jul 31, 2019

azclub commented Aug 1, 2019

bflad left a comment

ghost commented Aug 7, 2019

ghost commented Nov 2, 2019

	arn = "arn:aws:iam::aws:policy/service-role/AWSGlueServiceRole"
	arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AWSGlueServiceRole"

Glue Crawler: Add catalog target config #9430

Glue Crawler: Add catalog target config #9430

Conversation

azclub commented Jul 21, 2019 • edited Loading

Community Note

azclub commented Jul 31, 2019

bflad left a comment

Choose a reason for hiding this comment

bflad Jul 31, 2019

Choose a reason for hiding this comment

bflad Jul 31, 2019

Choose a reason for hiding this comment

bflad Jul 31, 2019

Choose a reason for hiding this comment

bflad Jul 31, 2019

Choose a reason for hiding this comment

azclub commented Aug 1, 2019

bflad left a comment

Choose a reason for hiding this comment

ghost commented Aug 7, 2019

ghost commented Nov 2, 2019

azclub commented Jul 21, 2019 •

edited

Loading