Azure / terraform-azurerm-avm-res-documentdb-databaseaccount

https://registry.terraform.io/modules/Azure/avm-res-documentdb-databaseaccount/azurerm/latest
MIT License
0 stars 0 forks source link

Feature/bryansan msft/new module sql #16

Closed bryansan-msft closed 2 months ago

bryansan-msft commented 3 months ago

Description

Type of Change

Checklist

bryansan-msft commented 3 months ago

Hello @Azure/avm-core-team-technical-terraform, let me know if this PR is ready to be published as the first version of this module

bryansan-msft commented 3 months ago

I disagree, this clunkiness affects every single customer/module just to support a few with weird edge cases LOL but okay, next page.

So, just to avoid another iteration of this since this is the 3rd time im being asked to do something with the key names and there is still not a unified criteria/guidance.


From: Matt White @.> Sent: Wednesday, May 22, 2024 5:36:58 PM To: Azure/terraform-azurerm-avm-res-documentdb-databaseaccount @.> Cc: Bryan Sanchez Pernia @.>; Mention @.> Subject: Re: [Azure/terraform-azurerm-avm-res-documentdb-databaseaccount] Feature/bryansan msft/new module sql (PR #16)

@matt-FFFFFF commented on this pull request.


In local.sql.tfhttps://github.com/Azure/terraform-azurerm-avm-res-documentdb-databaseaccount/pull/16#discussion_r1610222113:

@@ -0,0 +1,83 @@ +locals {

  • normalized_sql_databases = {
  • for db_key, db_params in var.sql_databases : coalesce(db_params.name, db_key) => db_params

The map key is arbitrary and not used in the resource. You should only reference each.key in a resource when you store data in maps with consistent keys.

I realise this way is more clunky. But... it is much more robust and is less prone to failures. As I have learned, robust TF modules aren't always the prettiest.

— Reply to this email directly, view it on GitHubhttps://github.com/Azure/terraform-azurerm-avm-res-documentdb-databaseaccount/pull/16#discussion_r1610222113, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A4BPQLV3YPSTNMNEMGW66ATZDS3RVAVCNFSM6AAAAABHJV2RW6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANZRGY2DMOBTG4. You are receiving this because you were mentioned.Message ID: @.***>

bryansan-msft commented 2 months ago

Hello @jaredfholgate, I have made all the changes you requested and the pipeline is in green. Please whenever you can I would appreciate an approve so I can close this chapter

bryansan-msft commented 2 months ago

@jaredfholgate @matt-FFFFFF ping?

matt-FFFFFF commented 2 months ago

LGTM. Thanks @bryansan-msft