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

Move users/databases to for_each #100

Merged
merged 6 commits into from
Apr 29, 2020

Conversation

Dev25
Copy link
Contributor

@Dev25 Dev25 commented Apr 2, 2020

^
Closes #96

Noticed a couple of object fields instance and project were never used so also removed them

To consider

  • Merge users and additonal_users + same for databases, seems a bit pointless having them seperate and since we've already done a breaking API change might as well merge them now.
  • Move read_replicas to for_each

Dev25 and others added 5 commits March 31, 2020 13:36
Signed-off-by: Dev <devan2patel@gmail.com>
Signed-off-by: Dev <devan2patel@gmail.com>
Signed-off-by: Dev <devan2patel@gmail.com>
Signed-off-by: Dev <devan@ingresso.co.uk>
Signed-off-by: Dev <devan@ingresso.co.uk>
@bharathkkb
Copy link
Member

Overall LGTM
Regarding points to consider, I havent used this module much but
Arent read_replicas defined under /modules/[mysql/pgsql]/read_replica.tf already using for_each

@Dev25
Copy link
Contributor Author

Dev25 commented Apr 22, 2020

Arent read_replicas defined under /modules/[mysql/pgsql]/read_replica.tf already using for_each

The replica configuration blocks are done via for_each but not the instance resource

resource "google_sql_database_instance" "replicas" {
  count                = var.read_replica_size

@Dev25
Copy link
Contributor Author

Dev25 commented Apr 22, 2020

I think for now lets ignore read replicas, imo that entire section could be more flexible e.g. different instance tiers/settings for different replicas so if/whenever that changes we could move to for_each then as well.

@syedrakib
Copy link

Do we have any ETA on this?

@bharathkkb
Copy link
Member

@Dev25 we have a conflict, can you rebase ?
@morgante PTAL

Signed-off-by: Dev <devan@ingresso.co.uk>
Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

Thank you and sorry for the delay.

@morgante morgante merged commit d433995 into terraform-google-modules:master Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move replica/users/databases to for_each
4 participants