search

dubiety / terraform-aws-elasticsearch-cloudwatch-sns-alarms

terraform module that configures important alarms for AWS elasticsearch and send them to SNS topic
Apache License 2.0
30 stars 45 forks source link

Added support for multi-node clusters to address issue #16. #17

Closed ericjsilva closed 2 years ago

dubiety commented 2 years ago

LGTM

AndrewFarley commented 2 years ago

Uh @dubiety is there a reason why we can't just always use "Sum" so we don't need to add a new variable for this? :\ Is there something I'm missing?

BTW, edited this, after further examination SUM is bad, I'm PR-ing removing this, and adding info to the documentation README.

dubiety commented 2 years ago

@AndrewFarley , I think you're right. This variable can be removed. I'll mark this variable as "deprecated". Will update this week. Thank you for the suggestion. CC @ericjsilva

ericjsilva commented 2 years ago

In researching this more, I agree with @AndrewFarley. I made some comments on the original issue I opened as well.

We should add clarity to the readme and variables.

AndrewFarley commented 2 years ago

I'll submit an PR for this then shortly @dubiety / @ericjsilva team.