-
Notifications
You must be signed in to change notification settings - Fork 203
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
Add support for SQL Database SKU and MaxSizeBytes #1235
Conversation
Seems to make sense 👍 |
f4d52e0
to
f685204
Compare
@matthchr because the spec changed here you will need to update the helm templates
then commit the changed files |
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.
need to update helm templates before merge
Probably should also update the sample manifest and/or the docs https://github.com/Azure/azure-service-operator/blob/master/config/samples/azure_v1beta1_azuresqldatabase.yaml |
return future.Response(), err | ||
} | ||
|
||
// AddLongTermRetention enables / disables long term retention | ||
func (_ *AzureSqlDbManager) AddLongTermRetention(ctx context.Context, resourceGroupName string, serverName string, databaseName string, weeklyRetention string, monthlyRetention string, yearlyRetention string, weekOfYear int32) (*http.Response, error) { | ||
|
||
longTermClient, err := azuresqlshared.GetBackupLongTermRetentionPoliciesClient() | ||
// TODO: Probably shouldn't return a response at all in the err case here (all through this function) |
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.
I think at times there have been attempts to make the Response more user friendly so we could rely on it for error handling. In the end we have had to just use data embedded in the error types.
pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go
Outdated
Show resolved
Hide resolved
f685204
to
0c71314
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.
can't require edition now
@@ -22,13 +49,16 @@ type AzureSqlDatabaseSpec struct { | |||
// +kubebuilder:validation:Required | |||
ResourceGroup string `json:"resourceGroup"` | |||
Server string `json:"server"` | |||
Edition DBEdition `json:"edition"` | |||
Edition DBEdition `json:"edition"` // TODO: Remove this in v1beta2 |
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.
this now needs to include omitempty
1cf6e02
to
77dbad2
Compare
77dbad2
to
d1878fd
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.
lgtm
Closes #1226
What this PR does / why we need it:
Allow customers full control over SQL server SKU and MaxSize
If applicable: