azurerm_mssql_server - updated to use parser #13151
Conversation
There was a problem hiding this comment.
Hi @catriona-m, this is looking great 👍
I made a couple of suggestions (the same thing in a few places) - the main one is that we can omit the Get request after creating/updating the resource since we already have the ID, and the other is making use of the String() method of the id struct to tidy up the error message strings.
If we can add those in, this should be good to merge 🚀
| if err != nil { | ||
| if !utils.ResponseWasNotFound(existing.Response) { | ||
| return fmt.Errorf("checking for presence of existing SQL Server %q (Resource Group %q): %+v", name, resGroup, err) | ||
| return fmt.Errorf("checking for presence of existing SQL Server %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) |
There was a problem hiding this comment.
To shorten this, we can use the String method from the id struct
| return fmt.Errorf("checking for presence of existing SQL Server %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) | |
| return fmt.Errorf("checking for presence of existing %s: %+v", id, err) |
| if err != nil { | ||
| return fmt.Errorf("issuing create/update request for SQL Server %q (Resource Group %q): %+v", name, resGroup, err) | ||
| return fmt.Errorf("issuing create/update request for SQL Server %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) |
There was a problem hiding this comment.
Also here
| return fmt.Errorf("issuing create/update request for SQL Server %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) | |
| return fmt.Errorf("creating/updating %s: %+v", id, err) |
| } | ||
|
|
||
| return fmt.Errorf("waiting on create/update future for SQL Server %q (Resource Group %q): %+v", name, resGroup, err) | ||
| return fmt.Errorf("waiting on create/update future for SQL Server %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) |
There was a problem hiding this comment.
| return fmt.Errorf("waiting on create/update future for SQL Server %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) | |
| return fmt.Errorf("waiting for creation/update of %s: %+v", id, err) |
| resp, err := client.Get(ctx, resGroup, name) | ||
| resp, err := client.Get(ctx, id.ResourceGroup, id.Name) | ||
| if err != nil { | ||
| return fmt.Errorf("issuing get request for SQL Server %q (Resource Group %q): %+v", name, resGroup, err) | ||
| return fmt.Errorf("issuing get request for SQL Server %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) | ||
| } |
There was a problem hiding this comment.
If we're only using this to retrieve the resource ID, we can forgoe the Get request and use the id struct to generate it instead
| if err != nil { | ||
| return fmt.Errorf("issuing get request for SQL Server %q (Resource Group %q): %+v", name, resGroup, err) | ||
| return fmt.Errorf("issuing get request for SQL Server %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) | ||
| } | ||
|
|
||
| d.SetId(*resp.ID) |
There was a problem hiding this comment.
e.g.
| d.SetId(*resp.ID) | |
| d.SetId(id.ID()) |
| if err != nil { | ||
| return fmt.Errorf("creating SQL Server %q AAD admin (Resource Group %q): %+v", name, resGroup, err) | ||
| return fmt.Errorf("creating SQL Server %q AAD admin (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) |
There was a problem hiding this comment.
This one could be tweaked to also use the id.String method
| return fmt.Errorf("creating SQL Server %q AAD admin (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) | |
| return fmt.Errorf("creating AAD admin for %s: %+v", id, err) |
| } | ||
|
|
||
| if err = adminFuture.WaitForCompletionRef(ctx, adminClient.Client); err != nil { | ||
| return fmt.Errorf("waiting for creation of SQL Server %q AAD admin (Resource Group %q): %+v", name, resGroup, err) | ||
| return fmt.Errorf("waiting for creation of SQL Server %q AAD admin (Resource Group %q): %+v", id.Name, id.Name, err) |
| if _, err = connectionClient.CreateOrUpdate(ctx, resGroup, name, connection); err != nil { | ||
| return fmt.Errorf("issuing create/update request for SQL Server %q Connection Policy (Resource Group %q): %+v", name, resGroup, err) | ||
| if _, err = connectionClient.CreateOrUpdate(ctx, id.ResourceGroup, id.Name, connection); err != nil { | ||
| return fmt.Errorf("issuing create/update request for SQL Server %q Connection Policy (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) |
| if err != nil { | ||
| return fmt.Errorf("issuing create/update request for SQL Server %q Blob Auditing Policies(Resource Group %q): %+v", name, resGroup, err) | ||
| return fmt.Errorf("issuing create/update request for SQL Server %q Blob Auditing Policies(Resource Group %q): %+v", id.Name, id.ResourceGroup, err) |
| } | ||
|
|
||
| if err = auditingFuture.WaitForCompletionRef(ctx, auditingClient.Client); err != nil { | ||
| return fmt.Errorf("waiting for creation of SQL Server %q Blob Auditing Policies(Resource Group %q): %+v", name, resGroup, err) | ||
| return fmt.Errorf("waiting for creation of SQL Server %q Blob Auditing Policies(Resource Group %q): %+v", id.Name, id.ResourceGroup, err) |
Thanks @manicminer, think I got them all! |
|
This functionality has been released in v2.75.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
No description provided.