cloudposse / terraform-aws-elasticache-redis

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

fix: add suffix the parameter group #144

Closed syphernl closed 2 years ago

syphernl commented 2 years ago

what

why

references

simoferr98 commented 2 years ago

Hi @syphernl I noticed this problem and I was about to open a new pull request.

The name of the resource aws_elasticache_parameter_group I would have indicated it like this:

name = "${module.this.id}-${random_id.redis_pg_name[0].hex}"

to keep the same name on the various resources.

Can you implement this change?

syphernl commented 2 years ago

Hi @syphernl I noticed this problem and I was about to open a new pull request.

The name of the resource aws_elasticache_parameter_group I would have indicated it like this:

name = "${module.this.id}-${random_id.redis_pg_name[0].hex}"

to keep the same name on the various resources.

Can you implement this change?

The prefix argument (docs) of the random_id resource should take care of that: https://github.com/cloudposse/terraform-aws-elasticache-redis/pull/144/files#diff-dc46acf24afd63ef8c556b77c126ccc6e578bc87e3aa09a931f33d9bf2532fbbR101. The generated name should return something like myproject-stage-redis-hex, which would only change again in case the description or family does.

simoferr98 commented 2 years ago

Hi @syphernl , Thanks for the clarification. I approve your changes

mergify[bot] commented 2 years ago

This pull request is now in conflict. Could you fix it @syphernl? 🙏

nitrocode commented 2 years ago

@syphernl is this still needed?

syphernl commented 2 years ago

No, we can close this one in favor of #143.