-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
mssql_managed_instance
- upgrade to 2023-08-01-preview
and hashicorp/go-azure-sdk
#26872
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @catriona-m - Just a few comments below (sorry if some are already outdated, I started the review before you pushed some changes)
internal/services/mssqlmanagedinstance/mssql_managed_database_resource.go
Outdated
Show resolved
Hide resolved
internal/services/mssqlmanagedinstance/mssql_managed_database_resource.go
Outdated
Show resolved
Hide resolved
func expandLongTermRetentionPolicy(ltrPolicy []LongTermRetentionPolicy) sql.BaseLongTermRetentionPolicyProperties { | ||
return sql.BaseLongTermRetentionPolicyProperties{ | ||
func expandLongTermRetentionPolicy(ltrPolicy []LongTermRetentionPolicy) managedinstancelongtermretentionpolicies.ManagedInstanceLongTermRetentionPolicyProperties { | ||
return managedinstancelongtermretentionpolicies.ManagedInstanceLongTermRetentionPolicyProperties{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably actually nil and length-check this here to avoid crashes in future? While it's fine in create, the update path could potentially have a change where the length is 0
. (The API might actually prevent this today, but we should be defensive against crashes that may be introduced in future)
internal/services/mssqlmanagedinstance/mssql_managed_database_resource.go
Outdated
Show resolved
Hide resolved
...vices/mssqlmanagedinstance/mssql_managed_instance_active_directory_administrator_resource.go
Outdated
Show resolved
Hide resolved
...vices/mssqlmanagedinstance/mssql_managed_instance_active_directory_administrator_resource.go
Outdated
Show resolved
Hide resolved
.../mssqlmanagedinstance/mssql_managed_instance_active_directory_administrator_resource_test.go
Outdated
Show resolved
Hide resolved
...vices/mssqlmanagedinstance/mssql_managed_instance_active_directory_administrator_resource.go
Outdated
Show resolved
Hide resolved
internal/services/mssqlmanagedinstance/mssql_managed_instance_resource.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes @catriona-m - LGTM 👍
a1b5994
to
5143edd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @catriona-m and @jackofallops LGTM 🌶️
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. |
Community Note
Description
This PR updates the
mssql_managed_instance
to2023-08-01-preview
. It also updates it to usehashicorp/go-azure-sdk
which is using the new base layer.To workaround the property name change from
StorageAccountType
toRequestedBackupStorageRedundancy
and the change in value names, this pr takes the old value names and maps them to the new names to avoid a breaking change.PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Testing
Tests running in TC
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
mssql_managed_instance
- upgrade to2023-08-01-preview
andhashicorp/go-azure-sdk
[GH-26872]This is a (please select all that apply):
Related Issue(s)
Fixes #0000
Note
If this PR changes meaningfully during the course of review please update the title and description as required.