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

enhancement(rds): Support default_only, include_all, and filter for aws_rds_engine_version #26923

Merged
merged 8 commits into from
Nov 9, 2022

Conversation

md5
Copy link
Contributor

@md5 md5 commented Sep 22, 2022

This PR adds support for default_only, include_all, and filter to the aws_rds_engine_version data source. Adding these flags make it possible to get the default minor version given only a major version for an engine, to distinguish between provisioned and serverless engine modes, or to allow querying of deprecated versions, among other things.

Closes #19715
Closes #26867

@github-actions
Copy link

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Please review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. size/S Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/rds Issues and PRs that pertain to the rds service. labels Sep 22, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @md5 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTOR guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@github-actions github-actions bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. 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. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Sep 22, 2022
@md5
Copy link
Contributor Author

md5 commented Sep 22, 2022

Added some tests for the new additions. Here is the output of make testacc:

% make testacc AWS_PROFILE=default TESTS=TestAccRDSEngineVersionDataSource PKG=rds
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/rds/... -v -count 1 -parallel 20 -run='TestAccRDSEngineVersionDataSource'  -timeout 180m
=== RUN   TestAccRDSEngineVersionDataSource_basic
=== PAUSE TestAccRDSEngineVersionDataSource_basic
=== RUN   TestAccRDSEngineVersionDataSource_upgradeTargets
=== PAUSE TestAccRDSEngineVersionDataSource_upgradeTargets
=== RUN   TestAccRDSEngineVersionDataSource_preferred
=== PAUSE TestAccRDSEngineVersionDataSource_preferred
=== RUN   TestAccRDSEngineVersionDataSource_defaultOnlyImplicit
=== PAUSE TestAccRDSEngineVersionDataSource_defaultOnlyImplicit
=== RUN   TestAccRDSEngineVersionDataSource_defaultOnlyExplicit
=== PAUSE TestAccRDSEngineVersionDataSource_defaultOnlyExplicit
=== RUN   TestAccRDSEngineVersionDataSource_includeAll
=== PAUSE TestAccRDSEngineVersionDataSource_includeAll
=== RUN   TestAccRDSEngineVersionDataSource_filter
=== PAUSE TestAccRDSEngineVersionDataSource_filter
=== CONT  TestAccRDSEngineVersionDataSource_basic
=== CONT  TestAccRDSEngineVersionDataSource_defaultOnlyExplicit
=== CONT  TestAccRDSEngineVersionDataSource_preferred
=== CONT  TestAccRDSEngineVersionDataSource_defaultOnlyImplicit
=== CONT  TestAccRDSEngineVersionDataSource_filter
=== CONT  TestAccRDSEngineVersionDataSource_includeAll
=== CONT  TestAccRDSEngineVersionDataSource_upgradeTargets
--- PASS: TestAccRDSEngineVersionDataSource_upgradeTargets (11.06s)
--- PASS: TestAccRDSEngineVersionDataSource_includeAll (11.07s)
--- PASS: TestAccRDSEngineVersionDataSource_filter (11.27s)
--- PASS: TestAccRDSEngineVersionDataSource_defaultOnlyExplicit (11.27s)
--- PASS: TestAccRDSEngineVersionDataSource_defaultOnlyImplicit (11.27s)
--- PASS: TestAccRDSEngineVersionDataSource_basic (11.35s)
--- PASS: TestAccRDSEngineVersionDataSource_preferred (11.59s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/rds	14.068s

@github-actions github-actions bot added the tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. label Sep 22, 2022
@md5 md5 force-pushed the rds-engine-version-enhancements branch from 2a952a6 to 25d7798 Compare September 29, 2022 03:55
@md5
Copy link
Contributor Author

md5 commented Oct 6, 2022

@justinretzolk do you have a sense for when the next round of PR triage will happen? I'd like to be able to plan for when this change might land in a release of the provider, particularly if I end up needing to address any feedback.

@breathingdust breathingdust 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 Oct 11, 2022
@md5 md5 force-pushed the rds-engine-version-enhancements branch from 25d7798 to c45bc58 Compare October 12, 2022 18:41
@md5
Copy link
Contributor Author

md5 commented Oct 13, 2022

@breathingdust I know I put enhancement in the title, but the original issue is marked as bug. Maybe this should be marked as bug too?

@md5
Copy link
Contributor Author

md5 commented Oct 26, 2022

Looks like this also closes #26867

@md5
Copy link
Contributor Author

md5 commented Oct 26, 2022

I just rebased this and reran the acceptance tests:

% TF_ACC=1 go test ./internal/service/rds/... -v -count 1 -parallel 20 -run='TestAccRDSEngineVersionDataSource'  -timeout 180m
=== RUN   TestAccRDSEngineVersionDataSource_basic
=== PAUSE TestAccRDSEngineVersionDataSource_basic
=== RUN   TestAccRDSEngineVersionDataSource_upgradeTargets
=== PAUSE TestAccRDSEngineVersionDataSource_upgradeTargets
=== RUN   TestAccRDSEngineVersionDataSource_preferred
=== PAUSE TestAccRDSEngineVersionDataSource_preferred
=== RUN   TestAccRDSEngineVersionDataSource_defaultOnlyImplicit
=== PAUSE TestAccRDSEngineVersionDataSource_defaultOnlyImplicit
=== RUN   TestAccRDSEngineVersionDataSource_defaultOnlyExplicit
=== PAUSE TestAccRDSEngineVersionDataSource_defaultOnlyExplicit
=== RUN   TestAccRDSEngineVersionDataSource_includeAll
=== PAUSE TestAccRDSEngineVersionDataSource_includeAll
=== RUN   TestAccRDSEngineVersionDataSource_filter
=== PAUSE TestAccRDSEngineVersionDataSource_filter
=== CONT  TestAccRDSEngineVersionDataSource_basic
=== CONT  TestAccRDSEngineVersionDataSource_filter
=== CONT  TestAccRDSEngineVersionDataSource_defaultOnlyExplicit
=== CONT  TestAccRDSEngineVersionDataSource_includeAll
=== CONT  TestAccRDSEngineVersionDataSource_preferred
=== CONT  TestAccRDSEngineVersionDataSource_defaultOnlyImplicit
=== CONT  TestAccRDSEngineVersionDataSource_upgradeTargets
--- PASS: TestAccRDSEngineVersionDataSource_filter (11.73s)
--- PASS: TestAccRDSEngineVersionDataSource_includeAll (12.00s)
--- PASS: TestAccRDSEngineVersionDataSource_defaultOnlyExplicit (12.34s)
--- PASS: TestAccRDSEngineVersionDataSource_basic (12.34s)
--- PASS: TestAccRDSEngineVersionDataSource_defaultOnlyImplicit (12.34s)
--- PASS: TestAccRDSEngineVersionDataSource_upgradeTargets (12.35s)
--- PASS: TestAccRDSEngineVersionDataSource_preferred (12.42s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/rds	14.939s

@justinretzolk
Copy link
Member

Hey @md5 👋 Thank you for your contribution, and for checking in on this! Unfortunately, I'm not able to provide an estimate on when this will be merged/reviewed due to the potential of shifting priorities (we prioritize work by count of ":+1:" reactions, as well as a few other things). For more information on how we prioritize, check out out prioritization guide.

@md5
Copy link
Contributor Author

md5 commented Nov 3, 2022

Hey @md5 👋 Thank you for your contribution, and for checking in on this! Unfortunately, I'm not able to provide an estimate on when this will be merged/reviewed due to the potential of shifting priorities (we prioritize work by count of "👍" reactions, as well as a few other things). For more information on how we prioritize, check out out prioritization guide.

That's all well and good, but I see a bunch of PRs opened since mine was opened that have no 👍 at all that have already been merged. For example, this one was opened and merged within the past 5 hours: #27620. It doesn't even have an open issue that it was addressing.

@justinretzolk
Copy link
Member

justinretzolk commented Nov 3, 2022

Hey @md5 👋 That's a great callout, and this is something that I'll start to think of how we can be a bit more clear about in the prioritization guide. I realize that you're making a more general statement and aren't pointing to that one PR in particular, but that one was contributed by a user who is in the top five all time contributors to the provider. It's sometimes the case that the team is able to review contributions from folks who contribute regularly a bit more quickly. This is also true with small documentation fixes -- I'm the Technical Community Manager for the AWS provider, and while doing my daily triage I'll often get those merged in almost immediately.

I saw that you also commented on #27342. I just wanted to mention that both the author and reviewer are community members like yourself, and that it has not been prioritized or reviewed by the AWS provider team at this point. I suspect that PR will be closed in favor of this one since you've added default_only and include_all as well, but I'd like to leave that to whoever picks this up for review once we're able to prioritize it.

@md5
Copy link
Contributor Author

md5 commented Nov 3, 2022

@justinretzolk thanks for the clarification on #27342. I should have noticed that the reviewer was not a project member. The fact that GitHub uses "Contributor" and "Member" with the same badge styling and prominence has a tendency to confuse me.

md5 added 2 commits November 8, 2022 14:02
Allows a major version to be specified for "version" and only get back a
single minor version
@md5 md5 force-pushed the rds-engine-version-enhancements branch from e3f2ae1 to 683de17 Compare November 8, 2022 22:03
Copy link
Member

@jar-b jar-b left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

$ make testacc PKG=rds TESTS=TestAccRDSEngineVersionDataSource
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/rds/... -v -count 1 -parallel 20 -run='TestAccRDSEngineVersionDataSource'  -timeout 180m
=== RUN   TestAccRDSEngineVersionDataSource_basic
=== PAUSE TestAccRDSEngineVersionDataSource_basic
=== RUN   TestAccRDSEngineVersionDataSource_upgradeTargets
=== PAUSE TestAccRDSEngineVersionDataSource_upgradeTargets
=== RUN   TestAccRDSEngineVersionDataSource_preferred
=== PAUSE TestAccRDSEngineVersionDataSource_preferred
=== RUN   TestAccRDSEngineVersionDataSource_defaultOnlyImplicit
=== PAUSE TestAccRDSEngineVersionDataSource_defaultOnlyImplicit
=== RUN   TestAccRDSEngineVersionDataSource_defaultOnlyExplicit
=== PAUSE TestAccRDSEngineVersionDataSource_defaultOnlyExplicit
=== RUN   TestAccRDSEngineVersionDataSource_includeAll
=== PAUSE TestAccRDSEngineVersionDataSource_includeAll
=== RUN   TestAccRDSEngineVersionDataSource_filter
=== PAUSE TestAccRDSEngineVersionDataSource_filter
=== CONT  TestAccRDSEngineVersionDataSource_basic
=== CONT  TestAccRDSEngineVersionDataSource_defaultOnlyExplicit
=== CONT  TestAccRDSEngineVersionDataSource_preferred
=== CONT  TestAccRDSEngineVersionDataSource_defaultOnlyImplicit
=== CONT  TestAccRDSEngineVersionDataSource_filter
=== CONT  TestAccRDSEngineVersionDataSource_includeAll
=== CONT  TestAccRDSEngineVersionDataSource_upgradeTargets
--- PASS: TestAccRDSEngineVersionDataSource_includeAll (15.65s)
--- PASS: TestAccRDSEngineVersionDataSource_upgradeTargets (15.72s)
--- PASS: TestAccRDSEngineVersionDataSource_filter (15.75s)
--- PASS: TestAccRDSEngineVersionDataSource_defaultOnlyExplicit (15.83s)
--- PASS: TestAccRDSEngineVersionDataSource_defaultOnlyImplicit (15.87s)
--- PASS: TestAccRDSEngineVersionDataSource_basic (15.97s)
--- PASS: TestAccRDSEngineVersionDataSource_preferred (16.72s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/rds        19.347s

@jar-b jar-b merged commit 44fd94d into hashicorp:main Nov 9, 2022
@jar-b
Copy link
Member

jar-b commented Nov 9, 2022

Thanks for your contribution @md5 👏

@github-actions github-actions bot added this to the v4.39.0 milestone Nov 9, 2022
@md5 md5 deleted the rds-engine-version-enhancements branch November 9, 2022 20:32
@github-actions
Copy link

This functionality has been released in v4.39.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 Dec 11, 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. enhancement Requests to existing resources that expand the functionality or scope. service/rds Issues and PRs that pertain to the rds service. size/M 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.

[Enhancement]: Add Filter to aws_rds_engine_version datasource aws_rds_engine_version failed
4 participants