.NET client update by stefanb995 · Pull Request #5197 · Azure/azure-sdk-for-net

stefanb995 · 2019-01-31T18:24:52Z

Adding two new parameters to Managed Instance (ProxyOverride and PublicDataEndpointEnabled)
Link to the PR with swagger change on which this code is generated: link

Description

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

Please add REST spec PR link to the SDK PR
I have read the contribution guidelines.
The pull request does not introduce breaking changes.

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 the commitid of your swagger spec or link to the swagger spec, used to generate the code.
The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

*Adding two new parameters to Managed Instance (ProxyOverride and PublicDataEndpointEnabled)

msftclas · 2019-01-31T18:25:04Z

All CLA requirements met.

dsgouda · 2019-02-01T18:52:11Z

@jaredmoo please approve before I post my review.

jaredmoo

Discussed offline - needs enum for proxy property.

stefanb995 · 2019-02-01T19:32:57Z

Here is a link for the PR with the swagger change and examples update.

jaredmoo

Looks good so far, please update at least 1 scenario test to use the new properties

Updated test to include parameters ProxyOverride and PublicDataEndpointEnabled

…m/stefanb995/azure-sdk-for-net into AddProxyOverrideAndPublicEndpoint

stefanb995 · 2019-02-07T16:04:40Z

Updated existing scenario test adding these two new parameters.

dsgouda · 2019-02-07T18:41:57Z

src/SDKs/SqlManagement/Management.Sql/Generated/SqlManagementClient.cs

-        /// <exception cref="System.ArgumentNullException">
-        /// Thrown when a required parameter is null
-        /// </exception>
-        public SqlManagementClient(ServiceClientCredentials credentials, HttpClient httpClient, bool disposeHttpClient) : this(httpClient, disposeHttpClient)


This is a breaking change, please bump version to 1.28.0-preview

Deepak, is this changed expected from latest autorest? Just want to check.

Stefan, when you update csproj with new version, please also wipe out old release notes from 1.27.0 and add your new release notes.

I'm not a 100% sure, but this is coming from AutoRest. @stefanb995 could you install latest autorest version and regen the code (autorest --reset)

src/SDKs/SDKs/_metadata/sql_resource-manager.txt

SDK ganerated with option -AutoRestVersion "latest" on master azure-rest- api-specs autorest version: 2.0.4283

…m/stefanb995/azure-sdk-for-net into AddProxyOverrideAndPublicEndpoint

stefanb995 · 2019-02-08T18:03:04Z

Hey guys, I have regenerated code from master branch.
autorest version is 2.0.4283.
There are some new files generated. Can you please take a look and tell me is this ok.

And deleted methods in SqlManagementClient from previous PR are back.

jaredmoo · 2019-02-08T18:25:39Z

Please update csproj with new version (1.28.0-preview) and please also wipe out old release notes from 1.27.0 and add your new release notes.

jaredmoo · 2019-02-08T18:29:54Z

Looks like you picked up sensitivity labels from @bashahee 's Swagger change ( Azure/azure-rest-api-specs#5148 ) , but we can't add that to SDK without tests. @dsgouda , what is your guidance when features get tied up like this?

stefanb995 · 2019-02-08T18:41:24Z

Updated csproj.

Please let me know what could I do on my side. Should I just delete these new files and everything added for sensitivity labels?

dsgouda · 2019-02-08T18:58:32Z

Looks like you picked up sensitivity labels from @bashahee 's Swagger change ( Azure/azure-rest-api-specs#5148 ) , but we can't add that to SDK without tests. @dsgouda , what is your guidance when features get tied up like this?

@shahabhijeet could you please take a look

dsgouda · 2019-02-08T22:51:11Z

@jaredmoo If I understand correctly, you are not ready to release the SesitivityLabels feature yet but you would like to release the changes in this PR.
The recommended way to do this is to remove the sensitivityLabel features from the default tag in readme.md so that generated code does not include that spec. Let us know if you have any doubts here

bashahee · 2019-02-10T08:13:02Z

@jaredmoo @dsgouda Can we please merge these clients? I updated readme.md and swagger files in order to generate clients (our GA is at end of the month), but I see that @stefanb995 already did that, so can our clients be merged along with current PR?

jaredmoo · 2019-02-10T16:39:14Z

@bashahee we need e2e scenario tests for all APIs, Can you please implement those?

bashahee · 2019-02-10T20:58:24Z

@bashahee we need e2e scenario tests for all APIs, Can you please implement those?

Yes, I will implement E2E scenario tests once this PR is merged. Is that what you meant?

jaredmoo · 2019-02-11T03:28:21Z

I mean can you make your own PR that has your own scenario tests? We can merge them both together. (I can help resolve merge during Redmond hours if needed)

stefanb995 · 2019-02-11T08:55:30Z

Guys, I need this merged as soon as possible. I need this change to be published to nuget before I send PR for powershell update.

dsgouda · 2019-02-11T16:50:30Z

@stefanb995 @jaredmoo please ensure that the code was regenerated after @bashahee 's changes so that this PR has exactly what you plan to publish in the SDK?
Also please fix the failing tests, please check if the tests are running locally in the Playback mode

dsgouda · 2019-02-11T16:51:23Z

@stefanb995 also link the Rest spec PR for which this SDK is being generated

stefanb995 · 2019-02-12T14:47:00Z

@dsgouda I generated this SDK on master branch of Rest spec. I have also attached PR for my Rest spec change.

I forgot to commit session record for the test I updated. I have updated it now. When I run tests in playback mode all tests are passing and 5 tests are being skipped.

dsgouda · 2019-02-12T17:02:40Z

@azuresdkci Test azure-sdk-for-net.client please

dsgouda · 2019-02-12T17:48:43Z

@azuresdkci retest this please

jaredmoo · 2019-02-12T23:51:34Z

We need to make sure that @bashahee has scenario tests for sensitivity labels before publishing

stefanb995 · 2019-02-13T09:09:42Z

Can @bashahee update AssemblyInfo.cs file with AssemblyFileVersion (1.28.0.0)?
And please let me know when we are ready to publish.

bashahee · 2019-02-13T13:42:16Z

@stefanb995 @jaredmoo
I opened a PR containing scenario tests for sensitivity labels clients.
#5240

Moreover, I updated AssemblyInfo.cs.

* .NET client update *Adding two new parameters to Managed Instance (ProxyOverride and PublicDataEndpointEnabled) * Delete build.err * Delete build.wrn * .NET client update * Update ManagedInstanceCrudScenarioTests.cs Updated test to include parameters ProxyOverride and PublicDataEndpointEnabled * Delete sql_resource-manager.txt * .NET sdk SDK ganerated with option -AutoRestVersion "latest" on master azure-rest- api-specs autorest version: 2.0.4283 * Update sql_resource-manager.txt * Update Microsoft.Azure.Management.Sql.csproj * Update of session record for updated test * Update TestCreateUpdateGetDropManagedInstance.json

.NET client update

da2758d

*Adding two new parameters to Managed Instance (ProxyOverride and PublicDataEndpointEnabled)

stefanb995 closed this Jan 31, 2019

stefanb995 reopened this Jan 31, 2019

stefanb995 closed this Jan 31, 2019

stefanb995 reopened this Feb 1, 2019

stefanb995 added 2 commits February 1, 2019 16:26

Delete build.err

16b1a1b

Delete build.wrn

1ca9c05

dsgouda assigned jaredmoo Feb 1, 2019

jaredmoo suggested changes Feb 1, 2019

View reviewed changes

stefanb995 closed this Feb 1, 2019

.NET client update

7dd1523

stefanb995 reopened this Feb 1, 2019

jaredmoo suggested changes Feb 1, 2019

View reviewed changes

Merge branch 'master' into AddProxyOverrideAndPublicEndpoint

c351c0a

dsgouda added the Changes Requested label Feb 5, 2019

stefanb995 added 2 commits February 7, 2019 17:01

Update ManagedInstanceCrudScenarioTests.cs

b1f09ce

Updated test to include parameters ProxyOverride and PublicDataEndpointEnabled

Merge branch 'AddProxyOverrideAndPublicEndpoint' of https://github.co…

a3f3975

…m/stefanb995/azure-sdk-for-net into AddProxyOverrideAndPublicEndpoint

stefanb995 closed this Feb 7, 2019

stefanb995 reopened this Feb 7, 2019

jaredmoo approved these changes Feb 7, 2019

View reviewed changes

dsgouda added the spec-not-merged label Feb 7, 2019

dsgouda suggested changes Feb 7, 2019

View reviewed changes

stefanb995 added 4 commits February 8, 2019 18:39

Delete sql_resource-manager.txt

411f99c

.NET sdk

964dea1

SDK ganerated with option -AutoRestVersion "latest" on master azure-rest- api-specs autorest version: 2.0.4283

Merge branch 'AddProxyOverrideAndPublicEndpoint' of https://github.co…

469b71d

…m/stefanb995/azure-sdk-for-net into AddProxyOverrideAndPublicEndpoint

Update sql_resource-manager.txt

581ef21

Update Microsoft.Azure.Management.Sql.csproj

a9763e2

stefanb995 added 2 commits February 12, 2019 15:42

Update of session record for updated test

84d3f2d

Update TestCreateUpdateGetDropManagedInstance.json

d2d0e35

dsgouda removed Changes Requested labels Feb 12, 2019

dsgouda approved these changes Feb 12, 2019

View reviewed changes

dsgouda merged commit b6b1a9f into Azure:master Feb 12, 2019

Comments

Conversation

stefanb995 commented Jan 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

General Guidelines

Testing Guidelines

SDK Generation Guidelines

Uh oh!

msftclas commented Jan 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dsgouda commented Feb 1, 2019

Uh oh!

jaredmoo left a comment

Choose a reason for hiding this comment

Uh oh!

stefanb995 commented Feb 1, 2019

Uh oh!

jaredmoo left a comment

Choose a reason for hiding this comment

Uh oh!

stefanb995 commented Feb 7, 2019

Uh oh!

dsgouda Feb 7, 2019

Choose a reason for hiding this comment

Uh oh!

jaredmoo Feb 7, 2019

Choose a reason for hiding this comment

Uh oh!

jaredmoo Feb 7, 2019

Choose a reason for hiding this comment

Uh oh!

dsgouda Feb 7, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stefanb995 commented Feb 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaredmoo commented Feb 8, 2019

Uh oh!

jaredmoo commented Feb 8, 2019

Uh oh!

stefanb995 commented Feb 8, 2019

Uh oh!

dsgouda commented Feb 8, 2019

Uh oh!

dsgouda commented Feb 8, 2019

Uh oh!

bashahee commented Feb 10, 2019

Uh oh!

jaredmoo commented Feb 10, 2019

Uh oh!

bashahee commented Feb 10, 2019

Uh oh!

jaredmoo commented Feb 11, 2019

Uh oh!

stefanb995 commented Feb 11, 2019

Uh oh!

dsgouda commented Feb 11, 2019

Uh oh!

dsgouda commented Feb 11, 2019

Uh oh!

stefanb995 commented Feb 12, 2019

Uh oh!

dsgouda commented Feb 12, 2019

Uh oh!

dsgouda commented Feb 12, 2019

Uh oh!

jaredmoo commented Feb 12, 2019

Uh oh!

stefanb995 commented Feb 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bashahee commented Feb 13, 2019

Uh oh!

Reviewers

Assignees

Labels

stefanb995 commented Jan 31, 2019 •

edited

Loading

msftclas commented Jan 31, 2019 •

edited

Loading

stefanb995 commented Feb 8, 2019 •

edited

Loading

stefanb995 commented Feb 13, 2019 •

edited

Loading