Skip to content

Feature/jobatzil/connection draining#2824

Merged
markcowl merged 16 commits intoAzure:AutoRestfrom
jobatzil:feature/jobatzil/connection_draining
Mar 4, 2017
Merged

Feature/jobatzil/connection draining#2824
markcowl merged 16 commits intoAzure:AutoRestfrom
jobatzil:feature/jobatzil/connection_draining

Conversation

@jobatzil
Copy link
Copy Markdown
Contributor

@jobatzil jobatzil commented Feb 15, 2017

Description

Added the support of ConnectionDraining for ApplicationGateway.

IMPORTANT
As can be seen in the PR linked below, we had to copy all files with no 2016-12-01 version yet to the 2016-12-01 folder to fix reference issues, which is why we had to adjust the test recordings of some files (in Network and Compute part).

Azure/azure-rest-api-specs#913

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

  • #[x] 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.

…ains only changes related to ConnectionDraining in ApplicationGateway and files which only change is the version number of the api calls.
… the network testrecordings from 2016-09-01 to 2016-12-01 using the script provided by Deepak (Because in azure-rest-api-specs we moved all missing files to the 2016-12-01 version folder, so the recordings needed to be adjusted!)
…e compute test recordings from 2016-09-01 to 2016-12-01 using the script provided by Deepak.
@azuresdkci
Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@azurecla
Copy link
Copy Markdown

Hi @jobatzil, 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 (jobatzil). 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;

@hovsepm
Copy link
Copy Markdown
Contributor

hovsepm commented Feb 15, 2017

@azuresdkci add to whitelist

@shahabhijeet
Copy link
Copy Markdown
Contributor

@jobatzil do any of your test ran on the CI? Does it run locally?

/// <param name="etag">A unique read-only string that changes whenever
/// the resource is updated.</param>
public ApplicationGatewayBackendHttpSettings(string id = default(string), int? port = default(int?), string protocol = default(string), string cookieBasedAffinity = default(string), int? requestTimeout = default(int?), SubResource probe = default(SubResource), IList<SubResource> authenticationCertificates = default(IList<SubResource>), string provisioningState = default(string), string name = default(string), string etag = default(string))
public ApplicationGatewayBackendHttpSettings(string id = default(string), int? port = default(int?), string protocol = default(string), string cookieBasedAffinity = default(string), int? requestTimeout = default(int?), SubResource probe = default(SubResource), IList<SubResource> authenticationCertificates = default(IList<SubResource>), string provisioningState = default(string), ApplicationGatewayConnectionDraining connectionDraining = default(ApplicationGatewayConnectionDraining), string name = default(string), string etag = default(string))
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.

@jobatzil any public surface area would require you to bump up major version due to breaking change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since this is a preview library, that is unnecessary

@jobatzil
Copy link
Copy Markdown
Contributor Author

Hey @shahabhijeet, yah all my tests were running.
Thanks for the catch though, all my prod traffic gets rerouted to next, that's why, so we should put that PR on hold until the changes get to Prod. Once the NRP rollout of that feature is out of next I'll ping you and we can proceed?

@shahabhijeet
Copy link
Copy Markdown
Contributor

@jobatzil can you close this PR and reopen when you are ready to merge. Also when do you think you will be ready to merge (approx. days this PR needs to be kept open)

@jobatzil jobatzil closed this Feb 21, 2017
@jobatzil jobatzil reopened this Mar 3, 2017
@@ -1092,4 +1092,4 @@
"Variables": {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

None of these changes that affect the line endings for compute json recordings should be there, pelase revery these

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.

done.

@markcowl markcowl dismissed shahabhijeet’s stale review March 4, 2017 01:27

Discussed offline, major version bump is not required

@markcowl markcowl merged commit 6dd4198 into Azure:AutoRest Mar 4, 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.

6 participants