Skip to content
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.

Sign up for GitHub

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

Machine Learning Compute: Add -IncludeAllResources Paramter for Remove cmdlet #5206

Merged
merged 9 commits into from
Jan 8, 2018
Merged

Machine Learning Compute: Add -IncludeAllResources Paramter for Remove cmdlet #5206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3cf2e88
Add IncludeAllResources for remove cmdlet
shutchings Dec 18, 2017
022d79d
Add local package while waiting for .net pr to be merged
shutchings Dec 20, 2017
22d18ba
Remove duplicate package references
shutchings Jan 2, 2018
129f3a0
Make remove should process message more explicit
shutchings Jan 2, 2018
d051479
Update changelog
shutchings Jan 3, 2018
e08231a
Remove local package
shutchings Jan 4, 2018
e683b93
Update breaking change exceptions for plural switch parameter
shutchings Jan 4, 2018
564b1dd
Use single parameter set attribute for parameter in all sets
shutchings Jan 5, 2018
e0a9dcc
Fix typo in help document
shutchings Jan 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/ResourceManager/MachineLearningCompute/ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
- Additional information about change #1
-->
## Current Release
* Add IncludeAllResources parameter to Remove-AzureRmMlOpCluster cmdlet
- Using this switch parameter will remove all resources that were created with the cluster originally
* Added Location Completer to -Location parameters allowing tab completion through valid Locations
* Added ResourceGroup Completer to -ResourceGroup parameters allowing tab completion through resource groups in current subscription

Expand Down
5 changes: 4 additions & 1 deletion ...gCompute/Commands.MachineLearningCompute.Test/Commands.MachineLearningCompute.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
<Private>True</Private>
</Reference>
<Reference Include="Microsoft.Azure.Management.MachineLearningCompute, Version=0.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\..\packages\Microsoft.Azure.Management.MachineLearningCompute.0.2.0\lib\net452\Microsoft.Azure.Management.MachineLearningCompute.dll</HintPath>
<HintPath>..\..\..\packages\Microsoft.Azure.Management.MachineLearningCompute.0.3.0\lib\net452\Microsoft.Azure.Management.MachineLearningCompute.dll</HintPath>
</Reference>
<Reference Include="Microsoft.Azure.Management.ResourceManager, Version=1.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\..\packages\Microsoft.Azure.Management.ResourceManager.1.6.0-preview\lib\net452\Microsoft.Azure.Management.ResourceManager.dll</HintPath>
Expand Down Expand Up @@ -173,6 +173,9 @@
<None Include="SessionRecords\Microsoft.Azure.Commands.MachineLearningCompute.Test.ScenarioTests.MLCTests\TestNewGetRemove.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Include="SessionRecords\Microsoft.Azure.Commands.MachineLearningCompute.Test.ScenarioTests.MLCTests\TestRemoveIncludeAllResources.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Include="SessionRecords\Microsoft.Azure.Commands.MachineLearningCompute.Test.ScenarioTests.MLCTests\TestSet.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
Expand Down
7 changes: 7 additions & 0 deletions ...ger/MachineLearningCompute/Commands.MachineLearningCompute.Test/ScenarioTests/MLCTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,12 @@ public void TestSet()
{
TestController.NewInstance.RunPsTest(this.interceptor, "Test-Set");
}

[Fact]
[Trait(Category.AcceptanceType, Category.CheckIn)]
public void TestRemoveIncludeAllResources()
{
TestController.NewInstance.RunPsTest(this.interceptor, "Test-RemoveIncludeAllResources");
}
}
}
27 changes: 25 additions & 2 deletions ...er/MachineLearningCompute/Commands.MachineLearningCompute.Test/ScenarioTests/MLCTests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Creates a local operationalization cluster for use in tests.
function GetDefaultLocalClusterProperties
{
$location = "East US 2 EUAP"
$clusterType = "West Central US"
$clusterType = "Local"
$description = "Deployed from powershell"

$cluster = New-Object Microsoft.Azure.Management.MachineLearningCompute.Models.OperationalizationCluster `
Expand Down Expand Up @@ -302,4 +302,27 @@ function Test-Set

# Cleanup
TeardownTest -ResourceGroupName $resourceGroupName
}
}

function Test-RemoveIncludeAllResources
{
$resourceGroupName = GetUniqueName("mlcrp-cmdlet-test-remove-all")
$clusterName = GetUniqueName("mlcrp-cmdlet-test-remove-all")

SetupTest $resourceGroupName

# Create the cluster
$cluster = GetDefaultLocalClusterProperties
$createdCluster = New-AzureRmMlOpCluster -ResourceGroupName $resourceGroupName -Name $clusterName -Cluster $cluster

# Get the managed by resource group name before deleting
$managedByResourceGroupName = GetManagedByResourceGroupName -ResourceGroupName $resourceGroupName

# Delete the cluster
Remove-AzureRmMlOpCluster -ResourceGroupName $resourceGroupName -Name $clusterName -IncludeAllResources

Assert-Throws ( Get-AzureRmResourceGroup -ResourceGroupName $managedByResourceGroupName )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this throw? Shouldn't it complete the operation successfully and delete both the managed resource group and the cluster? And if it is supposed to throw, I would think you'd want another test with the command running successfully.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This throws because after a successful remove call with -IncludeAllResources the resource group I'm trying to get should not exist. So by expecting the throw I can verify that the -IncludeAllResources worked correctly.

Copy link
Contributor

@maddieclayton maddieclayton Jan 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that makes sense - I did not expect that a Get would throw when the resource is not found, but if that is the behavior then this test makes sense. Thanks.


# Cleanup
Remove-AzureRmResourceGroup -ResourceGroupName $resourceGroupName -Force
}
332,291 changes: 260,011 additions & 72,280 deletions ...t.Azure.Commands.MachineLearningCompute.Test.ScenarioTests.MLCTests/TestNewGetRemove.json
View file
Open in desktop

Large diffs are not rendered by default.

Loading