cloudposse / terraform-aws-elasticache-redis

Terraform module to provision an ElastiCache Redis Cluster
https://cloudposse.com/accelerate
Apache License 2.0
141 stars 246 forks source link

add missing global_replication_group_id variable #205

Closed amitblumshtien closed 3 months ago

amitblumshtien commented 9 months ago

what

I added missing var global_replication_group_id so i will be able to create redis cluster and associate it to a data store.

why

missing functionality

references

(https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/elasticache_global_replication_group)

sahiljambhekar commented 5 months ago

Are there plans to merge this in at any point? It's a really small change, but allows the module users to then use it with elasticache_global_replication_group

amitblumshtien commented 5 months ago

waiting for the owners to approve it.

sahiljambhekar commented 5 months ago

@srhopkins could you pls review?

joe-niland commented 5 months ago

Hi @amitblumshtien Sorry for the delay on this. It looks fine but we need the update the docs.

Could you please run the following and commit the result?

make init
make github/init
make readme

You may also need to install gomplate: https://docs.gomplate.ca/installing/

sahiljambhekar commented 5 months ago

Are there plans to merge this in at any point? It's a really small change, but allows the module users to then use it with elasticache_global_replication_group

Actually, On second thought, this change won't work, as a Replication group that's part of a global datastore, can't set it's own engine version, cluster node type etc, among other things. Most values need to be set to null. I can create an issue describing it, and contribute that fix back, unless @amitblumshtien want's to take it up.

mergify[bot] commented 3 months ago

💥 This pull request now has conflicts. Could you fix it @amitblumshtien? 🙏

mergify[bot] commented 3 months ago

This PR was closed due to inactivity and merge conflicts. 😭 Please resolve the conflicts and reopen if necessary.