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

feat(elasticache-redis): add engine input for valkey support #1170

Merged
merged 3 commits into from
Oct 30, 2024

Conversation

nitrocode
Copy link
Member

@nitrocode nitrocode commented Oct 29, 2024

what

  • add engine input for valkey support

why

  • Valkey is far cheaper than redis
Notes
  • Design options
    1. Add a new key to local.cluster_attributes and add a variable to the component, default it to redis, and pass as an argument to the module as-is
      • We could shy away from the pattern to using engine = var.engine in the local.
    2. Allow var.redis_clusters to supply engine with a default for redis and pass to module as-is
    3. Same as option 2 but allow local.cluster_attributes to overwrite it
      • This might be best of both worlds however no other argument does this so it would be breaking the pattern
  • Went with option 2 so the argument isn't a new requirement for everyone and it doesn't break consistency

references

@nitrocode nitrocode requested review from a team as code owners October 29, 2024 01:01
Signed-off-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
@GabisCampana
Copy link

@Nuru

Copy link
Contributor

@Nuru Nuru left a comment

Choose a reason for hiding this comment

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

Please enumerate the legal values of engine in the variable description. Please also provide, in the README, an explanation of what they mean, with one or more links to even more information.

Co-authored-by: Nuru <Nuru@users.noreply.github.com>
@nitrocode nitrocode requested a review from Nuru October 29, 2024 20:18
@nitrocode
Copy link
Member Author

Please enumerate the legal values of engine in the variable description. Please also provide, in the README, an explanation of what they mean, with one or more links to even more information.

Thanks for reviewing. I'll add this now.

Signed-off-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
@nitrocode
Copy link
Member Author

@Nuru changes addressed, please re-review.

@Nuru Nuru merged commit 29f90e0 into cloudposse:main Oct 30, 2024
5 checks passed
@nitrocode nitrocode deleted the redis-engine branch November 5, 2024 10:48
goruha pushed a commit to cloudposse-terraform-components/aws-elasticache-redis that referenced this pull request Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants