subscription: exposing the id field
#10598
Conversation
There was a problem hiding this comment.
hey @stawii
Thanks for this PR.
Taking a look through this mostly LGTM - however can we rename this nested field to id rather than resource_manager_id?
Thanks!
| @@ -34,6 +34,11 @@ func dataSourceSubscriptions() *schema.Resource { | |||
| Computed: true, | |||
| Elem: &schema.Resource{ | |||
| Schema: map[string]*schema.Schema{ | |||
| "resource_manager_id": { | |||
There was a problem hiding this comment.
can we make this just id to match other resources:
| "resource_manager_id": { | |
| "id": { |
| @@ -99,6 +104,9 @@ func dataSourceSubscriptionsRead(d *schema.ResourceData, meta interface{}) error | |||
|
|
|||
| s := make(map[string]interface{}) | |||
|
|
|||
| if v := val.ID; v != nil { | |||
| s["resource_manager_id"] = *v | |||
There was a problem hiding this comment.
can we make this just id to match other resources:
| s["resource_manager_id"] = *v | |
| s["id"] = *v |
| @@ -17,6 +17,7 @@ func TestAccDataSourceSubscriptions_basic(t *testing.T) { | |||
| { | |||
| Config: SubscriptionsDataSource{}.basic(), | |||
| Check: resource.ComposeTestCheckFunc( | |||
| check.That(data.ResourceName).Key("subscriptions.0.resource_manager_id").Exists(), | |||
There was a problem hiding this comment.
can we make this just id to match other resources:
| check.That(data.ResourceName).Key("subscriptions.0.resource_manager_id").Exists(), | |
| check.That(data.ResourceName).Key("subscriptions.0.id").Exists(), |
| @@ -36,6 +36,7 @@ output "first_available_subscription_display_name" { | |||
|
|
|||
| The `subscription` block contains: | |||
|
|
|||
| * `resource_manager_id` - The Resource Manager ID of this subscription. | |||
There was a problem hiding this comment.
can we make this just id to match other resources:
| * `resource_manager_id` - The Resource Manager ID of this subscription. | |
| * `id` - The ID of this Subscription. |
There was a problem hiding this comment.
I was a bit confused if I should use id or resource_manager_id ;) id fits "better" and it compatible with azurerm_subscription, but I've seen resource_manager_id in a lot of different places too. Seems I've picked wrong.
I'll change it soon.
There was a problem hiding this comment.
no worries - we use resource_manager_id only where a secondary is present (e.g. data plane id) - otherwise this should be id
stawii
changed the title
resource_manager_id fieldid field
|
Tests pass:
|
|
This has been released in version 2.48.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 2.48.0"
}
# ... other configuration ... |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked as resolved and limited conversation to collaborators
Fixes #10093