new datasource azurerm_extended_location_custom_location
#28066
Conversation
github-actions
bot
added
bug
documentation
service/extended-location
size/XL
waiting-response
labels
|
|
There was a problem hiding this comment.
HI @teowa thanks for submitting this. I left a couple of comments inline for you to consider. I also wanted to note that we should create a new resource file for the updated name(extended_location_custom_location_resource.go) rather than pointing to the existing resource file. Thanks!
| var model CustomLocationResourceModel | ||
| if err := metadata.Decode(&model); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| subscriptionId := metadata.Client.Account.SubscriptionId | ||
| client := metadata.Client.ExtendedLocation.CustomLocationsClient |
There was a problem hiding this comment.
| var model CustomLocationResourceModel | |
| if err := metadata.Decode(&model); err != nil { | |
| return err | |
| } | |
| subscriptionId := metadata.Client.Account.SubscriptionId | |
| client := metadata.Client.ExtendedLocation.CustomLocationsClient | |
| client := metadata.Client.ExtendedLocation.CustomLocationsClient | |
| subscriptionId := metadata.Client.Account.SubscriptionId | |
| var model CustomLocationResourceModel | |
| if err := metadata.Decode(&model); err != nil { | |
| return err | |
| } |
| } | ||
| } | ||
|
|
||
| func (r CustomLocationDataSource) Attributes() map[string]*pluginsdk.Schema { |
There was a problem hiding this comment.
could these be ordered alphabetically?
| id := customlocations.NewCustomLocationID(subscriptionId, model.ResourceGroupName, model.Name) | ||
| resp, err := client.Get(ctx, id) | ||
| if err != nil { | ||
| return fmt.Errorf("reading %s: %+v", id, err) |
There was a problem hiding this comment.
| return fmt.Errorf("reading %s: %+v", id, err) | |
| return fmt.Errorf("retrieving %s: %+v", id, err) |
| ResourceGroupName: id.ResourceGroupName, | ||
| } | ||
| if model := resp.Model; model != nil { | ||
| state.Location = model.Location |
There was a problem hiding this comment.
| state.Location = model.Location | |
| state.Location = location.Normalize(model.Location) |
| state.HostType = string(pointer.From(props.HostType)) | ||
| state.Namespace = pointer.From(props.Namespace) | ||
|
|
||
| // API always returns an empty `authentication` block even it's not specified. Tracing the bug: https://github.com/Azure/azure-rest-api-specs/issues/30101 |
There was a problem hiding this comment.
we can remove this given that this is the datasource
.github/labeler-issue-triage.yml
Outdated
| @@ -144,7 +144,7 @@ service/event-hubs: | |||
| - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_eventhub((.|\n)*)###' | |||
|
|
|||
| service/extended-location: | |||
| - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_extended_custom_location((.|\n)*)###' | |||
| - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(extended_custom_location\W+|extended_location_custom_location\W+|extended_location_custom_location\W+)((.|\n)*)###' | |||
There was a problem hiding this comment.
Why extended_location_custom_location\W+ twice?
There was a problem hiding this comment.
The file is generated by the go generate ./internal/provider command. The duplicates should be handled within the internal/tools/generator-services/main.go file now.
…m into new_data_custom_location
|
|
There was a problem hiding this comment.
Hi @teowa thanks for making the requested changes on this. On having another look through, I spotted a few minor things but once those are addressed this should be good. Thanks!
| model := existing.Model | ||
|
|
||
| if model.Properties == nil { | ||
| return fmt.Errorf("retreiving properties for %s for update: %+v", *id, err) |
There was a problem hiding this comment.
| return fmt.Errorf("retreiving properties for %s for update: %+v", *id, err) | |
| return fmt.Errorf("retrieving properties for %s for update: %+v", *id, err) |
There was a problem hiding this comment.
could we retain the original tests in addition to new ones so we can ensure the original resource is still working as expected?
|
|
||
| props := customlocations.CustomLocation{ | ||
| Id: pointer.To(id.ID()), | ||
| Location: model.Location, |
There was a problem hiding this comment.
| Location: model.Location, | |
| Location: location.Normalize(model.Location), |
Community Note
Description
new datasource
azurerm_extended_location_custom_locationThis PR also fixes the inconsistency regarding the resource name
azurerm_extended_location_custom_locationbetween the code and the documentation:PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_extended_location_custom_locationThis is a (please select all that apply):
Related Issue(s)
Fixes #28744
Note
If this PR changes meaningfully during the course of review please update the title and description as required.