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

Resource aws athena workgroup #12254

Merged
merged 9 commits into from
Mar 25, 2020

Conversation

leppikallio
Copy link
Contributor

@leppikallio leppikallio commented Mar 4, 2020

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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates to #10085

Release note for CHANGELOG:

Adding recursive_delete_option for aws_athena_workgroup resource

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSAthenaWorkGroup'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -count 1 -parallel 20 -run=TestAccAWSAthenaWorkGroup -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSAthenaWorkGroup_basic
=== PAUSE TestAccAWSAthenaWorkGroup_basic
=== RUN   TestAccAWSAthenaWorkGroup_disappears
=== PAUSE TestAccAWSAthenaWorkGroup_disappears
=== RUN   TestAccAWSAthenaWorkGroup_Configuration_BytesScannedCutoffPerQuery
=== PAUSE TestAccAWSAthenaWorkGroup_Configuration_BytesScannedCutoffPerQuery
=== RUN   TestAccAWSAthenaWorkGroup_Configuration_EnforceWorkgroupConfiguration
=== PAUSE TestAccAWSAthenaWorkGroup_Configuration_EnforceWorkgroupConfiguration
=== RUN   TestAccAWSAthenaWorkGroup_Configuration_PublishCloudWatchMetricsEnabled
=== PAUSE TestAccAWSAthenaWorkGroup_Configuration_PublishCloudWatchMetricsEnabled
=== RUN   TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_EncryptionConfiguration_SseS3
=== PAUSE TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_EncryptionConfiguration_SseS3
=== RUN   TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_EncryptionConfiguration_Kms
=== PAUSE TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_EncryptionConfiguration_Kms
=== RUN   TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_OutputLocation
=== PAUSE TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_OutputLocation
=== RUN   TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_OutputLocation_ForceDestroy
=== PAUSE TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_OutputLocation_ForceDestroy
=== RUN   TestAccAWSAthenaWorkGroup_Description
=== PAUSE TestAccAWSAthenaWorkGroup_Description
=== RUN   TestAccAWSAthenaWorkGroup_State
=== PAUSE TestAccAWSAthenaWorkGroup_State
=== RUN   TestAccAWSAthenaWorkGroup_ForceDestroy
=== PAUSE TestAccAWSAthenaWorkGroup_ForceDestroy
=== RUN   TestAccAWSAthenaWorkGroup_Tags
=== PAUSE TestAccAWSAthenaWorkGroup_Tags
=== CONT  TestAccAWSAthenaWorkGroup_basic
=== CONT  TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_OutputLocation
=== CONT  TestAccAWSAthenaWorkGroup_Tags
=== CONT  TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_EncryptionConfiguration_SseS3
=== CONT  TestAccAWSAthenaWorkGroup_ForceDestroy
=== CONT  TestAccAWSAthenaWorkGroup_State
=== CONT  TestAccAWSAthenaWorkGroup_Description
=== CONT  TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_OutputLocation_ForceDestroy
=== CONT  TestAccAWSAthenaWorkGroup_Configuration_PublishCloudWatchMetricsEnabled
=== CONT  TestAccAWSAthenaWorkGroup_Configuration_BytesScannedCutoffPerQuery
=== CONT  TestAccAWSAthenaWorkGroup_disappears
=== CONT  TestAccAWSAthenaWorkGroup_Configuration_EnforceWorkgroupConfiguration
=== CONT  TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_EncryptionConfiguration_Kms
--- PASS: TestAccAWSAthenaWorkGroup_disappears (25.30s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_EncryptionConfiguration_SseS3 (29.73s)
--- PASS: TestAccAWSAthenaWorkGroup_basic (32.79s)
--- PASS: TestAccAWSAthenaWorkGroup_Description (45.51s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_PublishCloudWatchMetricsEnabled (47.53s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_EnforceWorkgroupConfiguration (47.57s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_BytesScannedCutoffPerQuery (50.91s)
--- PASS: TestAccAWSAthenaWorkGroup_ForceDestroy (57.47s)
--- PASS: TestAccAWSAthenaWorkGroup_Tags (59.53s)
--- PASS: TestAccAWSAthenaWorkGroup_State (61.78s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_OutputLocation (63.52s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_OutputLocation_ForceDestroy (64.33s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_EncryptionConfiguration_Kms (71.83s)
PASS

leppikallio added 7 commits March 1, 2020 22:01
… gen' command and commit.

GNUmakefile:36: recipe for target 'gencheck' failed
make: *** [gencheck] Error 1
The command "make gencheck" exited with 2.
… gen' command and commit.

GNUmakefile:36: recipe for target 'gencheck' failed
make: *** [gencheck] Error 1
The command "make gencheck" exited with 2.
@leppikallio leppikallio requested a review from a team March 4, 2020 17:46
@ghost ghost added size/M Managed by automation to categorize the size of a PR. needs-triage Waiting for first response or review from a maintainer. documentation Introduces or discusses updates to documentation. service/athena Issues and PRs that pertain to the athena service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Mar 4, 2020
@ewbankkit
Copy link
Contributor

@leppikallio @teraken0509 This looks like the same functionality as #10111 but implemented via the API RecursiveDeleteOption field.

@leppikallio
Copy link
Contributor Author

@leppikallio @teraken0509 This looks like the same functionality as #10111 but implemented via the API RecursiveDeleteOption field.

Oh, how embarrassing to even admit but I didn't notice the one and original from @teraken0509 which is obviously quite a bit more elegant way to achieve the goal.

Thank you @ewbankkit for pointing this out.

@ewbankkit
Copy link
Contributor

@leppikallio Using the API-provided mechanism is simpler and means less code to maintain over the long term. Maybe use some of the ideas from #10111 (e.g. name the attribute force_destroy as that seems to be a convention in other resources and reusesome of the acceptance test cases)?

…p#12254](hashicorp#12254),  changed the optional feature to `force_destroy`instead of `recursive_delete_option`. Added test case inspired by @teraken0509, [resource_aws_athena_workgroup_test.go](https://github.com/teraken0509/terraform-provider-aws/blob/feature/add-force_destroy-attribute-for-aws_athena_workgroup-resource/aws/resource_aws_athena_workgroup_test.go)

Updated the documentation / help page to reflect the change.
@ghost ghost added size/L Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Mar 6, 2020
@leppikallio
Copy link
Contributor Author

Based on comments from @ewbankkit I changed the optional feature to force_destroy instead of recursive_delete_option.

Added test case inspired by @teraken0509 , resource_aws_athena_workgroup_test.go

Updated the documentation / help page to reflect the change.

@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Mar 25, 2020
@bflad bflad self-assigned this Mar 25, 2020
@bflad bflad added this to the v2.55.0 milestone Mar 25, 2020
@bflad bflad linked an issue Mar 25, 2020 that may be closed by this pull request
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @leppikallio 🚀

Output from acceptance testing:

--- PASS: TestAccAWSAthenaWorkGroup_disappears (13.52s)
--- PASS: TestAccAWSAthenaWorkGroup_basic (17.71s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_PublishCloudWatchMetricsEnabled (20.62s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_EncryptionConfiguration_SseS3 (21.41s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_EnforceWorkgroupConfiguration (23.30s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_BytesScannedCutoffPerQuery (35.04s)
--- PASS: TestAccAWSAthenaWorkGroup_Description (35.46s)
--- PASS: TestAccAWSAthenaWorkGroup_ForceDestroy (36.53s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_OutputLocation (37.56s)
--- PASS: TestAccAWSAthenaWorkGroup_Tags (39.00s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_EncryptionConfiguration_Kms (41.25s)
--- PASS: TestAccAWSAthenaWorkGroup_Configuration_ResultConfiguration_OutputLocation_ForceDestroy (50.81s)
--- PASS: TestAccAWSAthenaWorkGroup_State (51.30s)

@bflad bflad merged commit 5178a03 into hashicorp:master Mar 25, 2020
bflad added a commit that referenced this pull request Mar 25, 2020
@ghost
Copy link

ghost commented Mar 27, 2020

This has been released in version 2.55.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
Copy link

ghost commented Apr 24, 2020

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!

@ghost ghost locked and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/athena Issues and PRs that pertain to the athena service. size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add force_destroy option to aws_athena_workgroup
3 participants