-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Add new AzureRM.Sql cmdlets [Get/Set]-AzureRmSqlDatabaseBackupShortTermRetentionPolicy #6326
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 new AzureRM.Sql cmdlets [Get/Set]-AzureRmSqlDatabaseBackupShortTermRetentionPolicy #6326
Conversation
…d Add-AzureBatchComputeNodeServiceLogs cmdlets
…unts on getting PoolNodeCounts
| [Parameter(ParameterSetName = PolicyByResourceServerDatabaseSet, | ||
| Mandatory = true, | ||
| ValueFromPipelineByPropertyName = true, | ||
| ValueFromPipeline = true, |
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.
Usually the atribute parameter ValueFromPipeline is used to bind by complex types.
public string ServerName { get; set; }
For string type you may want to keep using ValueFromPipelineByPropertyName.
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.
Agh, I switched this when I was playing around with it and forgot to switch back. Thanks for the catch!
| ValueFromPipelineByPropertyName = true, | ||
| ValueFromPipeline = true, | ||
| Position = 2, | ||
| HelpMessage = "The name of the Azure SQL Database to use.")] |
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.
the same as above.
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.
Sure
| HelpMessage = "The database object to get the policy for.")] | ||
| [ValidateNotNullOrEmpty] | ||
| public AzureSqlDatabaseModel InputObject { get; set; } | ||
|
|
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.
Usually the InputObject name is uses for parameter of the same type as the cmdlet output type.
In this case you may want to rename the parameter from InputObject to AzureSqlDatabase, for example.
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.
Will do
| ``` | ||
| Get-AzureRmSqlDatabaseBackupShortTermRetentionPolicy -ResourceId <String> [-ResourceGroupName] <String> | ||
| [-DefaultProfile <IAzureContextContainer>] [<CommonParameters>] | ||
| ``` |
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.
[-ResourceGroupName] is useless here.
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.
Updating help docs again.
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.
the same for Set-
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.
Done
|
Is there a reason this code is no longer in the latest release of AzureRm.Sql, 4.12.1? I really need this method, and I can't find it in the latest release. |
|
@dvmorris it is currently only in the preview releases. I'll move it over to our release branch today! |
|
@dealaus thanks! That would be great. It's the only way I can find to actually set the PITR days value dynamically from PowerShell, and we need to do it as part of a regulatory requirement. |
|
@dvmorris No problem. I will let you know when my pull request goes through, but if you'd like to be unblocked before our next release of the Az.Sql module, you can temporarily use the latest preview module which has the cmdlet. |
Description
This change introduces 2 new cmdlets:
Cmdlet review: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/54
I made small modifications to what was reviewed (renamed AzureRmSqlDatabaseShortTermRetentionPolicy->AzureRmSqlDatabaseBackupShortTermRetentionPolicy) to better align with resource names and existing cmdlets.
Additionally, I added a temporary validation of the -RetentionDays param to enforce that the value is a multiple of 7 (we decided we only want retention to be in weeks for now). This validation is not deployed WW server-side yet, so I am doing the validation client-side for now and I will remove once the server validation is gone.
Checklist
CONTRIBUTING.mdplatyPSmodule