-
Notifications
You must be signed in to change notification settings - Fork 2
Get Container and Unregister Container for AzureSql #324
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -163,6 +163,10 @@ public static ContainerType GetPsContainerType(string containerType) | |
{ | ||
return ContainerType.Windows; | ||
} | ||
else if (containerType == ContainerType.AzureSqlContainer.ToString()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this comparison be done against the corresponding ContainerType of the Hydra DLL? |
||
{ | ||
return ContainerType.AzureSqlContainer; | ||
} | ||
else | ||
{ | ||
throw new Exception("Unsupported ContainerType: " + containerType); | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -414,7 +414,7 @@ Please contact Microsoft for further assistant.</value> | |
<value>Please provide AzureRmBackupManagementServer of BackupEngineType as DpmEngine or DpmVenusEngine and provide BackupManagementType as SCDPM. Provided AzureRmBackupManagementServer has BackupEngineType {0} and backupManagementType {1} which is not valid.</value> | ||
</data> | ||
<data name="UnsupportedContainerException" xml:space="preserve"> | ||
<value>Please provide Container of containerType as Windows and backupManagementType as MARS. Provided Container has containerType {0} and backupManagementType {1} which is invalid.</value> | ||
<value>Please provide Container of containerType as Windows and backupManagementType as MARS or Container of containerType as AzureSQL and backupManagementType as AzureSQL. Provided Container has containerType {0} and backupManagementType {1} which is invalid.</value> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please consider something like: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not taking this one. Scrubbing will finally happen by PM |
||
</data> | ||
<data name="GetRPErrorInputDatesShouldBeInUTC" xml:space="preserve"> | ||
<value>Please specify startdate and enddate in UTC format</value> | ||
|
@@ -447,6 +447,6 @@ Please contact Microsoft for further assistant.</value> | |
<value>BackupManagementType provider for ContainerType {0} is incorrect.</value> | ||
</data> | ||
<data name="AllowedSqlRetentionRange" xml:space="preserve"> | ||
<value>RetentionCount in Weeks should be 1-520, in Months should be 1-120 and Years should be 1-10</value> | ||
<value>Retention count in Weeks should be in between 1-520, in Months should be in between 1-120 and Years should be in between 1-10</value> | ||
</data> | ||
</root> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -188,7 +188,25 @@ public ProtectionPolicyResponse ModifyPolicy() | |
|
||
public List<Models.AzureRmRecoveryServicesBackupContainerBase> ListProtectionContainers() | ||
{ | ||
throw new NotImplementedException(); | ||
string name = (string)this.ProviderData.ProviderParameters[ContainerParams.Name]; | ||
|
||
ProtectionContainerListQueryParams queryParams = new ProtectionContainerListQueryParams(); | ||
|
||
queryParams.BackupManagementType = Microsoft.Azure.Management.RecoveryServices.Backup.Models.BackupManagementType.AzureSql.ToString(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use alias instead of referencing the complete namespace here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't alias be used if it is used in multiple places? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's what my comment was about :) Please use alias instead of the complete namespace |
||
|
||
var listResponse = HydraAdapter.ListContainers(queryParams); | ||
|
||
List<AzureRmRecoveryServicesBackupContainerBase> containerModels = ConversionHelpers.GetContainerModelList(listResponse); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please break this line. We shouldn't have lines that are too long. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand how can this be broken further ??? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe something like: |
||
|
||
if (!string.IsNullOrEmpty(name)) | ||
{ | ||
if (containerModels != null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the containerModels object ever be null? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In null case it will just return null which means 0 number of objects back There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
{ | ||
containerModels = containerModels.Where(x => x.Name == name).ToList(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use better names. Consider using "containerModel" instead of "x". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lambda expression are best used with such variables. because it is understood here that x here means containerModels. do let me know if you disagree There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. |
||
} | ||
} | ||
|
||
return containerModels; | ||
} | ||
|
||
public List<AzureRmRecoveryServicesBackupEngineBase> ListBackupManagementServers() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,12 +41,14 @@ public override void ExecuteCmdlet() | |
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic appears to be quite specific to the workload. Shouldn't this be in the corresponding provider for a cleaner implementation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this need to move to provider model in next iteration |
||
base.ExecuteCmdlet(); | ||
|
||
if (Container.ContainerType != ContainerType.Windows || Container.BackupManagementType != BackupManagementType.MARS) | ||
if (!((Container.ContainerType == ContainerType.Windows && Container.BackupManagementType == BackupManagementType.MARS) || | ||
(Container.ContainerType == ContainerType.AzureSqlContainer && Container.BackupManagementType == BackupManagementType.AzureSql))) | ||
{ | ||
throw new ArgumentException(String.Format(Resources.UnsupportedContainerException, Container.ContainerType, Container.BackupManagementType)); | ||
} | ||
AzureRmRecoveryServicesMabContainer mabContainer = Container as AzureRmRecoveryServicesMabContainer; | ||
string containerName = mabContainer.Name; | ||
string containerName = Container.Name; | ||
if (Container.ContainerType == ContainerType.AzureSqlContainer) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use braces There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do this change |
||
containerName = ContainerConstansts.SqlContainerNamePrefix + containerName; | ||
HydraAdapter.UnregisterContainers(containerName); | ||
}); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,4 +24,9 @@ public class PolicyConstants | |
public const int MinPolicyNameLength = 3; | ||
public const int MaxPolicyNameLength = 150; | ||
} | ||
|
||
public class ContainerConstansts | ||
{ | ||
public const string SqlContainerNamePrefix = "AzureSqlContainer;"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This prefix seems to be ContainerType + ";". Isn't it better to use the ContainerType itself rather than create a new constant for better code manageability? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will happen in next iteration. |
||
} | ||
} |
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.
put some trace if no type is is matched.