search

hashicorp / terraform-aws-consul

A Terraform Module for how to run Consul on AWS using Terraform and Packer
Apache License 2.0
401 stars 484 forks source link

add subnet ids variables #115

Closed davidbennatan closed 5 years ago

davidbennatan commented 5 years ago

add subnet ids variables

davidbennatan commented 5 years ago

I have a vpc with both private and public networks, so i need the option to chose the elastic group subnet location. I test on my private enviroment on my fork, with both inserted and default arguments

brikis98 commented 5 years ago

OK, important sanity check: as mentioned here, here, and here the code in the root of this module is example code. It is meant to show one simple way to use the modules in the modules folder and not to be used directly in prod. I appreciate the PR, and the changes you propose are reasonable, but they sound like you are trying to use the code in the root directly for a real use-case...

davidbennatan commented 5 years ago

You right, my environment is not yet prod, but i should switch to use the inner modules directly. nonetheless I think that the changes will slightly increase the example code flexibility

brikis98 commented 5 years ago

I think that the changes will slightly increase the example code flexibility

True, but at the cost of making the examples harder to read, maintain, and test. If you think this is a critical change to have, I can merge it in, but otherwise, I'm not sure it's worth the extra overhead.

davidbennatan commented 5 years ago

Well, I cant really call this change critical, I should just switch and use the inner modules directly as I supposed to. thanks for the review.