Skip to content

Go-VCR: support elasticache #43201

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Go-VCR: support elasticache #43201

wants to merge 1 commit into from
+1,145 −1,172

Conversation

jar-b
Copy link
Member

@jar-b jar-b commented Jun 26, 2025

Rollback Plan

If a change needs to be reverted, we will publish an updated version of the library.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

Description

Enables Go-VCR in the elasticache service.

Relations

Relates #25602

Output from Acceptance Testing

% VCR_MODE=REPLAY_ONLY VCR_PATH=/Users/jaredbaker/development/_worktrees/td-vcr-elasticache/testdata/ make testacc PKG=elasticache TESTS=TestAccElastiCacheCluster_Engine_redis
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.24.4 test ./internal/service/elasticache/... -v -count 1 -parallel 20 -run='TestAccElastiCacheCluster_Engine_redis'  -timeout 360m -vet=off
2025/06/26 15:19:45 Creating Terraform AWS Provider (SDKv2-style)...
2025/06/26 15:19:45 Initializing Terraform AWS Provider (SDKv2-style)...
=== RUN   TestAccElastiCacheCluster_Engine_redis
=== PAUSE TestAccElastiCacheCluster_Engine_redis
=== RUN   TestAccElastiCacheCluster_Engine_redis_v5
=== PAUSE TestAccElastiCacheCluster_Engine_redis_v5
=== CONT  TestAccElastiCacheCluster_Engine_redis
=== CONT  TestAccElastiCacheCluster_Engine_redis_v5
    cluster_test.go:156: stopping VCR recorder
    cluster_test.go:156: persisting randomness seed
--- PASS: TestAccElastiCacheCluster_Engine_redis_v5 (10.12s)
=== NAME  TestAccElastiCacheCluster_Engine_redis
    cluster_test.go:84: stopping VCR recorder
    cluster_test.go:84: persisting randomness seed
--- PASS: TestAccElastiCacheCluster_Engine_redis (10.15s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/elasticache        15.943s

@github-actions GitHub Actions
Copy link

Community Guidelines

This comment is added to every new Pull Request to provide quick reference to how the Terraform AWS Provider is maintained. Please review the information below, and thank you for contributing to the community that keeps the provider thriving! 🚀

Voting for Prioritization

  • Please vote on this Pull Request by adding a 👍 reaction to the original post to help the community and maintainers prioritize it.
  • Please see our prioritization guide for additional information on how the maintainers handle prioritization.
  • Please do not leave +1 or other comments that do not add relevant new information or questions; they generate extra noise for others following the Pull Request and do not help prioritize the request.

Pull Request Authors

  • 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 prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/elasticache Issues and PRs that pertain to the elasticache service. size/XL Managed by automation to categorize the size of a PR. labels Jun 26, 2025
ewbankkit
ewbankkit reviewed Jun 26, 2025
View reviewed changes
internal/service/elasticache/cluster.go
Comment on lines -507 to +508
if !d.IsNewResource() && tfresource.NotFound(err) {
if !d.IsNewResource() && retry.NotFound(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking now it's probably best to keep this as tfresource.NotFound, at least until we are sure nothing uses the SDKv2 helper/retry package.

ewbankkit
ewbankkit reviewed Jun 26, 2025
View reviewed changes
internal/service/elasticache/cluster.go
Comment on lines -790 to +792
if tfresource.TimedOut(err) {
if retry.TimedOut(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking now it's probably best to keep this as tfresource.TimedOut, at least until we are sure nothing uses the SDKv2 helper/retry package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ewbankkit ewbankkit ewbankkit left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. service/elasticache Issues and PRs that pertain to the elasticache 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jar-b @ewbankkit