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

add quicksight data source resource #20710

Closed

Conversation

lacernetic
Copy link
Contributor

@lacernetic lacernetic commented Aug 27, 2021

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

NOTE: This code is an updated version of issue #11094 created by @mjgpy3. Due to his absence I will take up role in changing it according to the code review.

Relates #10990
Closes #11094

Output from acceptance testing:

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSQuickSightDataSource -timeout 180m
=== RUN   TestAccAWSQuickSightDataSource_basic
=== PAUSE TestAccAWSQuickSightDataSource_basic
=== RUN   TestAccAWSQuickSightDataSource_disappears
=== PAUSE TestAccAWSQuickSightDataSource_disappears
=== CONT  TestAccAWSQuickSightDataSource_basic
=== CONT  TestAccAWSQuickSightDataSource_disappears
--- PASS: TestAccAWSQuickSightDataSource_disappears (36.73s)
--- PASS: TestAccAWSQuickSightDataSource_basic (41.33s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       41.466s
$ make testacc TESTARGS='-run=TestAccAWSQuickSightDataSource'

...

@github-actions github-actions bot added documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/quicksight Issues and PRs that pertain to the quicksight service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. needs-triage Waiting for first response or review from a maintainer. size/XL Managed by automation to categorize the size of a PR. labels Aug 27, 2021
@lacernetic lacernetic changed the title WIP: add quicksight data source resource [WIP] add quicksight data source resource Aug 27, 2021
@lacernetic lacernetic marked this pull request as draft August 27, 2021 17:09
@breathingdust breathingdust added new-data-source Introduces a new data source. and removed needs-triage Waiting for first response or review from a maintainer. labels Aug 27, 2021
@lacernetic lacernetic marked this pull request as ready for review September 16, 2021 22:13
@lacernetic lacernetic changed the title [WIP] add quicksight data source resource add quicksight data source resource Sep 16, 2021
@lacernetic lacernetic marked this pull request as draft September 16, 2021 22:14
@github-actions github-actions bot removed the provider Pertains to the provider itself, rather than any interaction with AWS. label Sep 17, 2021
@github-actions github-actions bot added the provider Pertains to the provider itself, rather than any interaction with AWS. label Sep 20, 2021
@lacernetic lacernetic marked this pull request as ready for review September 21, 2021 17:35
@lacernetic
Copy link
Contributor Author

I noticed that this PR has a label of "data-source", it can be a little confusing but this is a resource called "data-source"

@breathingdust breathingdust removed the new-data-source Introduces a new data source. label Sep 21, 2021
@breathingdust breathingdust added the new-resource Introduces a new resource. label Sep 21, 2021
@anGie44 anGie44 self-assigned this Sep 27, 2021
@anGie44
Copy link
Contributor

anGie44 commented Sep 29, 2021

Hi @lacernetic, thank you again for this PR and extending the initial work submitted by @mjgpy3 ! Overall the implementation is in a good place. To get this into the upcoming release, I'm going to refactor some bits to align with our contributing practices and then this should be good to go. The first of which will be Tags handling; the methods added in this PR are the right idea, however, we have methods available to us in the provider which we can substitute in; for future reference https://github.com/hashicorp/terraform-provider-aws/blob/main/docs/contributing/contribution-checklists.md#resource-tagging-code-implementation. The second refactoring will occur in method names tied to expanding/flattening API objects; we'll want to follow this guidance https://github.com/hashicorp/terraform-provider-aws/blob/main/docs/contributing/data-handling-and-conversion.md#recommended-implementations. The third addition will then be custom waiter logic; as seen in the resource's Read CRUD operation, some retry handling can be necessary to ensure the resource is in a ready state. To generalize the retry logic for create and update operations, custom waiters will be added per https://github.com/hashicorp/terraform-provider-aws/blob/main/docs/contributing/retries-and-waiters.md#resource-lifecycle-retries . For the unit testing, I'll move that over to the specific resource test file and then add a tags related acceptance test as well.

Do you mind toggling Allow edits from maintainers for this PR when you have a chance?

@anGie44 anGie44 added the waiting-response Maintainers are waiting on response from community or contributor. label Sep 29, 2021
@lacernetic
Copy link
Contributor Author

@anGie44 Hey Angie! I totally want to allow edits for maintainers, but I'm struggling to find the option for it. I went to github's guide on doing this, but I don't see that option anywhere. I am pretty confident that my organization is not interfering with this. Do you know of another way to do this?

@github-actions github-actions bot removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 29, 2021
@anGie44
Copy link
Contributor

anGie44 commented Sep 29, 2021

Hmm I would of shared the same link for reference 😅 No worries though! I can create a new PR that retains all of your commits and then push changes there.

Update: OK PR available at #21093 👍

@github-actions github-actions bot added this to the v3.61.0 milestone Sep 30, 2021
@github-actions
Copy link

github-actions bot commented Oct 7, 2021

This functionality has been released in v3.61.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. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 17, 2022
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. new-resource Introduces a new resource. provider Pertains to the provider itself, rather than any interaction with AWS. service/quicksight Issues and PRs that pertain to the quicksight service. size/XL 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.

3 participants