Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding LTR for MI to .NET SDK #10129

Merged
merged 19 commits into from
Mar 3, 2020
Merged

Adding LTR for MI to .NET SDK #10129

merged 19 commits into from
Mar 3, 2020

Conversation

xaliciayang
Copy link
Contributor

Adding clients + test file for Managed Instance Long Term Retention Tests

@isra-fel isra-fel self-assigned this Feb 25, 2020
@isra-fel isra-fel added Mgmt This issue is related to a management-plane library. needs-review and removed needs-review labels Feb 25, 2020
Copy link
Member

@isra-fel isra-fel left a comment

Choose a reason for hiding this comment

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

Hi @xaliciayang , it failed to build because some of the models could not be found, e.g. ManagedInstanceLongTermRetentionBackup. Did you forget to check in them?

Besides, please

  • add link(s) to the swagger spec review in this PR's description
  • check in the metadata txt of your module under /eng/mgmt/mgmtmetadata

Detailed process guide: https://github.com/Azure/adx-documentation-pr/blob/master/engineering/adx_netsdk_process.md#generating-net-sdk

@isra-fel isra-fel assigned xaliciayang and unassigned isra-fel Feb 25, 2020
@xaliciayang
Copy link
Contributor Author

Added the missing models (also added many unrelated files in attempt to sync local repo). Will remove out-of-scope files

Here is the swagger PR: Azure/azure-rest-api-specs#8266

/// <exception cref="Microsoft.Rest.ValidationException">
/// Thrown when a required parameter is null
/// </exception>
Task<AzureOperationResponse<IPage<ManagedDatabase>>> ListInaccessibleByInstanceWithHttpMessagesAsync(string resourceGroupName, string managedInstanceName, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken));
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not our change

/// <exception cref="Microsoft.Rest.ValidationException">
/// Thrown when a required parameter is null
/// </exception>
Task<AzureOperationResponse<IPage<ManagedDatabase>>> ListInaccessibleByInstanceNextWithHttpMessagesAsync(string nextPageLink, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken));
Copy link
Contributor

Choose a reason for hiding this comment

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

not our change

"no-cache"
],
"Location": [
"https://api-dogfood.resources.windows-int.net/subscriptions/8cfb8b62-bcd6-4713-89ad-18097f75cc5b/providers/Microsoft.Sql/locations/southeastasia/managedDatabaseOperationResults/1dd83660-8f4f-4079-952d-2fb139812065?api-version=2019-06-01-preview"
Copy link
Contributor

Choose a reason for hiding this comment

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

are we ok to expose test endpoint?

/// <summary>
/// Defines values for DatabaseState6.
/// </summary>
public static class DatabaseState6
Copy link
Contributor

Choose a reason for hiding this comment

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

Autorest bug??

@@ -87,7 +89,7 @@ internal static partial class SdkInfo
new Tuple<string, string, string>("Sql", "RestorePoints", "2017-03-01-preview"),
new Tuple<string, string, string>("Sql", "SensitivityLabels", "2017-03-01-preview"),
new Tuple<string, string, string>("Sql", "ServerAutomaticTuning", "2017-03-01-preview"),
new Tuple<string, string, string>("Sql", "ServerAzureADAdministrators", "2014-04-01"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amolagar should I revert this and leave it to you?

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

Choose a reason for hiding this comment

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

why is this removed?

/// </param>
/// <param name='disposeHttpClient'>
/// True: will dispose the provided httpClient on calling SqlManagementClient.Dispose(). False: will not dispose provided httpClient</param>
protected SqlManagementClient(HttpClient httpClient, bool disposeHttpClient) : base(httpClient, disposeHttpClient)
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this removed?

@brjohnstmsft brjohnstmsft removed their request for review March 3, 2020 01:46
@@ -110,19 +112,10 @@ internal static partial class SdkInfo
new Tuple<string, string, string>("Sql", "Usages", "2018-06-01-preview"),
new Tuple<string, string, string>("Sql", "VirtualClusters", "2015-05-01-preview"),
new Tuple<string, string, string>("Sql", "VirtualNetworkRules", "2015-05-01-preview"),
new Tuple<string, string, string>("Sql", "WorkloadClassifiers", "2019-06-01-preview"),
Copy link
Contributor

Choose a reason for hiding this comment

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

remove workloadClassifiers

@xaliciayang xaliciayang changed the title adding LTR MI tests Adding LTR for MI to .NET SDK Mar 3, 2020
@isra-fel isra-fel merged commit 77bf34a into Azure:master Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mgmt This issue is related to a management-plane library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants