Skip to content

Update SDK based on examples changes to swagger spec#2847

Merged
shahabhijeet merged 24 commits intoAzure:AutoRestfrom
nathannfan:AutoRest
Feb 28, 2017
Merged

Update SDK based on examples changes to swagger spec#2847
shahabhijeet merged 24 commits intoAzure:AutoRestfrom
nathannfan:AutoRest

Conversation

@nathannfan
Copy link
Copy Markdown
Contributor

Description

Swagger spec pull request: Azure/azure-rest-api-specs#954

This change updated the auto-generated SDK based on changes to the swagger spec that were done in response to examples validation and Fiddler testing.

This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

SDK Generation Guidelines

  • If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
  • The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as a link to the swagger spec, used to generate the code.
  • The project.json and AssemblyInfo.cs files have been updated with the new version of the SDK.

@azurecla
Copy link
Copy Markdown

Hi @nathannfan, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!


It looks like you're working at Microsoft (nafan). If you're full-time, we DON'T require a contribution license agreement.



If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

string testName = this.GetType().FullName;
SqlManagementTestUtilities.RunTestInNewV12Server(testName, "TestDatabasePointInTimeRestore", testPrefix, (resClient, sqlClient, resourceGroup, server) =>
{
Dictionary<string, string> tags = new Dictionary<string, string>()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Remove this

@nathannfan nathannfan self-assigned this Feb 24, 2017
activity = sqlClient.ElasticPools.ListDatabaseActivity(resourceGroup.Name, server.Name, epName2);
Assert.Equal(2, activity.Where(a => a.DatabaseName == dbName).Count());
Assert.Equal(1, activity.Where(a => a.DatabaseName == dbName && a.Operation == "CREATE").Count());
Assert.Equal(1, activity.Where(a => a.DatabaseName == dbName && a.Operation == "UPDATE").Count());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you also test removing the database from the pool?

Copy link
Copy Markdown
Contributor

@jaredmoo jaredmoo left a comment

Choose a reason for hiding this comment

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

Looks great, I would just like to see a test to remove db from pool.

@jaredmoo
Copy link
Copy Markdown
Contributor

Approved

@nathannfan nathannfan removed the request for review from akromm-zz February 25, 2017 01:21
Copy link
Copy Markdown
Contributor

@shahabhijeet shahabhijeet left a comment

Choose a reason for hiding this comment

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

What version of AutoRest used to generate code.
Was the latest version used.
Also I don't see name simplification was used during code generation.

throw new Microsoft.Rest.SerializationException("Unable to deserialize the response.", _responseContent, ex);
}
}
// Deserialize Response
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nathannfan did the swagger changed and is no longer returning 202?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Swagger still documents 202, but no longer has a response body, which I believe is why this section is removed.

/// A response object containing the response body and response headers.
/// </return>
public async System.Threading.Tasks.Task<Microsoft.Rest.Azure.AzureOperationResponse<System.Collections.Generic.IEnumerable<ElasticPoolDatabaseActivity>>> ListDatabaseActivityWithHttpMessagesAsync(string elasticPoolName, string resourceGroupName, string serverName, System.Collections.Generic.Dictionary<string, System.Collections.Generic.List<string>> customHeaders = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken))
public async System.Threading.Tasks.Task<Microsoft.Rest.Azure.AzureOperationResponse<System.Collections.Generic.IEnumerable<ElasticPoolDatabaseActivity>>> ListDatabaseActivityWithHttpMessagesAsync(string resourceGroupName, string serverName, string elasticPoolName, System.Collections.Generic.Dictionary<string, System.Collections.Generic.List<string>> customHeaders = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nathannfan was name simplification used when the code was generated via AutoRest?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@shahabhijeet Not sure how that is specified. Is there documentation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Synced up with Cormac. This requires a later version of AutoRest.

@cormacpayne
Copy link
Copy Markdown
Member

@azuresdkci test this please

@shahabhijeet
Copy link
Copy Markdown
Contributor

@nathannfan starting next PR, please use the latest Autorest 1.0 to regen code. It's ok for this PR.

@shahabhijeet shahabhijeet merged commit 55c8ea6 into Azure:AutoRest Feb 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants