[RecoveryServices.Backup] Added SoftDelete Feature for IaaSVM

[sambitratha]

Description

Cmdlet Review : https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/378

Checklist

• I have read the Submitting Changes section of CONTRIBUTING.md
• The title of the PR is clear and informative
• The appropriate ChangeLog.md file(s) has been updated:
  • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
  • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
• The PR does not introduce breaking changes
• If applicable, the changes made in the PR have proper test coverage
• For public API changes to cmdlets:
  • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
  • the markdown help files have been regenerated using the commands listed here

[adxsdkps]

Can one of the admins verify this patch?

[sambitratha]

Not able to identify the static Analysis error. Can you please help? Thanks

[markcowl]

It shows up in the artifacts of the job, the issue is here:

D:\a\1\s\artifacts\Debug\Az.RecoveryServices\Microsoft.Azure.PowerShell.Cmdlets.RecoveryServices.Backup.dll | Microsoft.Azure.Commands.RecoveryServices.Backup.Cmdlets.UndeleteAzureRmRecoveryServicesBackupItem | Undelete-AzRecoveryServicesBackupItem | 1 | 8300 | Undelete-AzRecoveryServicesBackupItem   uses the verb 'Undelete', which is not on the list of approved verbs for   PowerShell commands. Use the cmdlet 'Get-Verb' to see the full list of   approved verbs and consider renaming the cmdlet. | Consider renaming the cmdlet to use an   approved verb for PowerShell.
-- | -- | -- | -- | -- | -- | --

Looks like you are adding a new cmdlet that is using an unapproved verb. Have you had a cmdlet review?

[pvrk]

@markcowl : I have reached out earlier via email for a cmdlet review. Please let me know when can we meet to review this.

[markcowl]

@pvrk Please submit a cmdlet review here: https://github.com/azure/azure-powershell-cmdlet-review-pr/issues We will schedule a meeting if it is necessary for the change

[siddharth7]

apart from minor fixes, LGTM

[msJinLei]

Can it be set as a default implementation so that no duplicate codes in the derived classes?

[sambitratha]

these are provider classes which implement interface methods. So it is not possible to implement a default interface method implementation

[markcowl]

I may be missing somethign about the implementation, but it looks like your code would throw a NotImplementedException to the user for most of your backup providers. You need to prevent this.

[markcowl]

So, what is tp prevent this from calling one of the unimplemented methods for UndeleteProtection, given that you only have one class that implements this.

At least, you need to catch the NotImplementedException that would occur here for most of your providers and return a message letting the user know that this is not supported.. At best, you should have another indicator of whether the method is implemented (for example, having a separate interface for undelete, or making the method implenent a TryUndeleteMethod that returns false if Undelete is not possible, rather than throwing an uncaught exception)..

[sambitratha]

This is the coding style we are following as of now. There are other features as well which are only enabled for some specific provider. For example, RegisterContainer() method is only implemented for Workload Provider. If called for other providers, the same behavior is expected. i.e. to throw NotImplementedException

[siddharth7]

@markcowl The undelete feature for other workloads will arrive as and when they are supported by our service which is planned to come in the near future.

[markcowl]

This is not an acceptable customer experience. You must make the change descried above in order to have this PR merged.

[sambitratha]

Build is failing only in linux environment. Not able to identify the error. Can anyone help? Thanks