Update Azurestack DiskRP admin SDK#4644
Conversation
| } | ||
| } | ||
|
|
||
| var migrationId = "ba0644a4-c2ed-4e3c-a167-089a32865297"; // System.Guid.NewGuid().ToString(); This guid should be the same as the ones in sessionRecord |
There was a problem hiding this comment.
Remove 'System.Guid.NewGuid().StoString();' from comment.
There was a problem hiding this comment.
sure. thanks.
| <VersionPrefix>0.2.0-preview</VersionPrefix> | ||
| <VersionPrefix>0.3.0-preview</VersionPrefix> | ||
| <AssemblyName>Microsoft.AzureStack.Management.Compute.Admin</AssemblyName> | ||
| <PackageTags>Microsoft Azure Stack;Compute;REST HTTP client;azureostackfficial;netcore451511</PackageTags> |
There was a problem hiding this comment.
You might want to add 'Managed Disks' to the PackageTags. Add others if you feel necessary.
| @@ -0,0 +1,62 @@ | |||
| | |||
There was a problem hiding this comment.
Can you do tests that should fail? For instance try to migrate to a share that doesn't exist or with a disk that is invalid?
There was a problem hiding this comment.
Try Assert.False or Assert.Exception constructs.
If you wish to add the tests but enable them at a later time, you could always skip the tests, hope this helps
There was a problem hiding this comment.
I have added more tests.
| <AssemblyName>Compute.Admin.Tests</AssemblyName> | ||
| <VersionPrefix>1.0.0</VersionPrefix> | ||
| </PropertyGroup> | ||
| <!--<PropertyGroup> |
There was a problem hiding this comment.
Please remove this change. Our tests are now running against netcoreapp2.0
There was a problem hiding this comment.
You are saying they should leave this commented out? If so they should just delete this XML entry then.
There was a problem hiding this comment.
Leave it commented for now, we will do a cleanup throughout the repo at a later point.
There was a problem hiding this comment.
OK. I will leave it as what it is.
dsgouda
left a comment
dsgouda
left a comment
There was a problem hiding this comment.
Looks great apart from the minor comment
|
@dsgouda We would like tests that verify invalid input. |
|
Will review the rest upon @deathly809 's approval |
|
@dsgouda Looks good to me! :) |
dsgouda
left a comment
dsgouda
left a comment
There was a problem hiding this comment.
Looks great apart from a minor comment. Won't block if you choose to integrate it later
| public void ValidateExpectedReturnCode(Action action, HttpStatusCode httpResponseCode) | ||
| { | ||
| try { action.Invoke(); } | ||
| try { |
There was a problem hiding this comment.
nit: Highly recommend using Assert.Exception here. Take a look at this
There was a problem hiding this comment.
Sure. Updated. Thanks.
3 participants
Description
Update Azurestack DiskRP admin SDK - compute module. It includes two kinds of resources:
a. Disk - Get and List
b. DiskMigrationJob - Get, List, Create and Stop.
PR for Swagger:
Azure/azure-rest-api-specs#3418
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Testing Guidelines
SDK Generation Guidelines
*.csprojandAssemblyInfo.csfiles have been updated with the new version of the SDK.