Add failover group operations #3155
Conversation
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
|
@btasdoven, |
btasdoven
requested review from
jaredmoo,
nathannfan and
shahabhijeet
and removed request for
nathannfan
| cancellationToken.ThrowIfCancellationRequested(); | ||
| string _responseContent = null; | ||
| if ((int)_statusCode != 201 && (int)_statusCode != 202) | ||
| if ((int)_statusCode != 200 && (int)_statusCode != 202) |
There was a problem hiding this comment.
Note to reviewers: Swagger change was Azure/azure-rest-api-specs#1166
| { | ||
| ReadOnlyEndpoint = new FailoverGroupReadOnlyEndpoint() | ||
| { | ||
| FailoverPolicy = "Disabled", |
There was a problem hiding this comment.
You should be able to write ReadOnlyEndpointFailoverPolicy.Disabled. (How do I know? see https://github.com/Azure/azure-rest-api-specs/blob/master/arm-sql/2015-05-01-preview/swagger/failoverGroups.json#L490 )
| }, | ||
| ReadWriteEndpoint = new FailoverGroupReadWriteEndpoint() | ||
| { | ||
| FailoverPolicy = "Automatic", |
There was a problem hiding this comment.
Use ReadWriteEndpointFailoverPolicy.Automatic
| } | ||
|
|
||
| [Fact] | ||
| public void TestUpdateFailoverGroup() |
There was a problem hiding this comment.
Seems a bit repetitive, could this be part of the above test case?
There was a problem hiding this comment.
Or could you at least factor out the common setup code?
There was a problem hiding this comment.
Combined the test cases into one.
| var targetDatabase = sqlClient.Databases.Get(resourceGroup.Name, partnerServer.Name, databaseName); | ||
| Assert.NotNull(sourceDatabase.FailoverGroupId); | ||
| Assert.NotNull(targetDatabase.FailoverGroupId); | ||
|
|
There was a problem hiding this comment.
Could you have a test where you call Failover? Seems like an important scenario for FailoverGroups ;)
There was a problem hiding this comment.
It doesn't have to be a separate test, could be part of this or the other one. As long as it's covered somehow.
There was a problem hiding this comment.
That makes sense :) Added failover operation as part of the test case.
| { | ||
| public class FailoverGroupCrudScenarioTests | ||
| { | ||
| public static string DefaultPrimaryLocation |
There was a problem hiding this comment.
Could you add these to SqlManagementTestUtilities?
I'm guessing they're for stage, maybe make them DefaultStageLocation and DefaultStageSecondaryLocation?
There was a problem hiding this comment.
Yup, they are for Stage. Moved them to SqlManagementTestUtilities.
| } | ||
| } | ||
|
|
||
| private Server CreateServer(SqlManagementClient sqlClient, ResourceGroup resourceGroup, string serverPrefix, string location) |
There was a problem hiding this comment.
This would be useful in SqlManagementTestUtilities as well, could you put it there?
| } | ||
| } | ||
|
|
||
| private Server CreateServer(SqlManagementClient sqlClient, ResourceGroup resourceGroup, string serverPrefix, string location) |
There was a problem hiding this comment.
Could you also move this to SqlManagementTestUtilities?
| // | ||
| var failoverGroupOnSecondary = sqlClient.FailoverGroups.Get(resourceGroup.Name, sourceServer.Name, failoverGroupName); | ||
| Assert.Equal(FailoverGroupReplicationRole.Secondary, failoverGroupOnSecondary.ReplicationRole); | ||
| Assert.Equal(FailoverGroupReplicationRole.Primary, failoverGroupOnSecondary.PartnerServers.First().ReplicationRole); |
shahabhijeet
left a comment
shahabhijeet
left a comment
There was a problem hiding this comment.
I don't see any change in version.
Will need changes to csproj and assemblyinfo files related to versions.
|
@azuresdkci add to whitelist |
|
@shahabhijeet, do you have any other comments? |
6 participants
Adding Failover Groups operations to Microsoft.Sql provider with their tests, and reflecting the API changes in our swagger file.
This is a breaking change in terms of Failover Groups APIs; however, this should not be an issue since the feature has not gone public.
Description
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Testing Guidelines
SDK Generation Guidelines
project.jsonandAssemblyInfo.csfiles have been updated with the new version of the SDK.