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

[MetricsAdvisor] Remove unnecessary ClientOptions type #16436

Merged
merged 4 commits into from
Oct 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion sdk/metricsadvisor/Azure.AI.MetricsAdvisor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,21 @@
- Added a public setter to `DataFeed.Options`.

### Breaking Changes
- In `MetricsAdvisorClient`, updated `CreateDataFeed` and `CreateDataFeedAsync` to take a whole `DataFeed` object as a parameter.
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed it. It should be MetricsAdvisorAdministrationClient.

- In `MetricsAdvisorClient`, changed return types of sync and async methods `GetIncidentRootCauses`, `GetMetricEnrichedSeriesData`, and `GetMetricSeriesData` to pageables.
- In `MetricsAdvisorAdministrationClient`, updated `CreateDataFeed` and `CreateDataFeedAsync` to take a whole `DataFeed` object as a parameter.
- In `MetricsAdvisorAdministrationClient`, changed return types of sync and async methods `GetAnomalyAlertConfigurations` and `GetMetricAnomalyDetectionConfigurations` to pageables.
- In `MetricsAdvisorAdministrationClient`, renamed parameter `alertConfigurationId` to `detectionConfigurationId` in sync and async `GetAnomalyAlertConfigurations` methods.
- In `MetricEnrichedSeriesData`, made elements of `ExpectedValues`, `Periods`, `IsAnomaly`, `LowerBoundaries` and `UpperBoundaries` nullables.
- Removed `MetricsAdvisorClientOptions` and `MetricsAdvisorAdministrationOptions` and replaced both with `MetricsAdvisorClientsOptions`.
- Renamed `MetricDimension` to `DataFeedDimension`.
- Renamed `DataAnomaly` to `DataPointAnomaly`.
- Renamed `IncidentStatus` to `AnomalyIncidentStatus`.
- Renamed `AlertingHook`, `EmailHook`, and `WebHook` to `NotificationHook`, `EmailNotificationHook`, and `WebNotificationHook`, respectively.
- Renamed `TimeMode` to `AlertQueryTimeMode`.

### Key Bug Fixes
- Fixed a bug in `GetMetricEnrichedSeriesData` sync and async methods where a `NullReferenceException` was thrown if a returned data point had missing data.
Copy link
Member Author

Choose a reason for hiding this comment

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

Missed this in an older PR. Fixed by:

In MetricEnrichedSeriesData, made elements of ExpectedValues, Periods, IsAnomaly, LowerBoundaries and UpperBoundaries nullables.


## 1.0.0-beta.1 (2020-10-08)

This is the first beta of the `Azure.AI.MetricsAdvisor` client library.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ public partial class MetricsAdvisorClient
{
protected MetricsAdvisorClient() { }
public MetricsAdvisorClient(System.Uri endpoint, Azure.AI.MetricsAdvisor.MetricsAdvisorKeyCredential credential) { }
public MetricsAdvisorClient(System.Uri endpoint, Azure.AI.MetricsAdvisor.MetricsAdvisorKeyCredential credential, Azure.AI.MetricsAdvisor.MetricsAdvisorClientOptions options) { }
public MetricsAdvisorClient(System.Uri endpoint, Azure.AI.MetricsAdvisor.MetricsAdvisorKeyCredential credential, Azure.AI.MetricsAdvisor.MetricsAdvisorClientsOptions options) { }
Copy link
Member

Choose a reason for hiding this comment

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

The plural Clients seems odd to me. Should we just keep the name the same and delete the other one?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see in the linked issue that this is the guideline 😞

Copy link
Member

Choose a reason for hiding this comment

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

omg I did the same. I was like.. ugh that name looks weird.... looked at guidelines.. ohh nooo :(.
Oh well, sad approve

public virtual Azure.Response<Azure.AI.MetricsAdvisor.Models.MetricFeedback> CreateMetricFeedback(Azure.AI.MetricsAdvisor.Models.MetricFeedback feedback, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual System.Threading.Tasks.Task<Azure.Response<Azure.AI.MetricsAdvisor.Models.MetricFeedback>> CreateMetricFeedbackAsync(Azure.AI.MetricsAdvisor.Models.MetricFeedback feedback, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Pageable<Azure.AI.MetricsAdvisor.Models.AnomalyAlert> GetAlerts(string alertConfigurationId, Azure.AI.MetricsAdvisor.Models.GetAlertsOptions options, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
Expand Down Expand Up @@ -36,10 +36,10 @@ public MetricsAdvisorClient(System.Uri endpoint, Azure.AI.MetricsAdvisor.Metrics
public virtual Azure.Pageable<string> GetValuesOfDimensionWithAnomalies(string detectionConfigurationId, string dimensionName, Azure.AI.MetricsAdvisor.Models.GetValuesOfDimensionWithAnomaliesOptions options, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.AsyncPageable<string> GetValuesOfDimensionWithAnomaliesAsync(string detectionConfigurationId, string dimensionName, Azure.AI.MetricsAdvisor.Models.GetValuesOfDimensionWithAnomaliesOptions options, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
}
public partial class MetricsAdvisorClientOptions : Azure.Core.ClientOptions
public partial class MetricsAdvisorClientsOptions : Azure.Core.ClientOptions
{
public MetricsAdvisorClientOptions(Azure.AI.MetricsAdvisor.MetricsAdvisorClientOptions.ServiceVersion version = Azure.AI.MetricsAdvisor.MetricsAdvisorClientOptions.ServiceVersion.V1_0) { }
public Azure.AI.MetricsAdvisor.MetricsAdvisorClientOptions.ServiceVersion Version { get { throw null; } }
public MetricsAdvisorClientsOptions(Azure.AI.MetricsAdvisor.MetricsAdvisorClientsOptions.ServiceVersion version = Azure.AI.MetricsAdvisor.MetricsAdvisorClientsOptions.ServiceVersion.V1_0) { }
public Azure.AI.MetricsAdvisor.MetricsAdvisorClientsOptions.ServiceVersion Version { get { throw null; } }
public enum ServiceVersion
{
V1_0 = 1,
Expand All @@ -58,7 +58,7 @@ public partial class MetricsAdvisorAdministrationClient
{
protected MetricsAdvisorAdministrationClient() { }
public MetricsAdvisorAdministrationClient(System.Uri endpoint, Azure.AI.MetricsAdvisor.MetricsAdvisorKeyCredential credential) { }
public MetricsAdvisorAdministrationClient(System.Uri endpoint, Azure.AI.MetricsAdvisor.MetricsAdvisorKeyCredential credential, Azure.AI.MetricsAdvisor.MetricsAdvisorClientOptions options) { }
public MetricsAdvisorAdministrationClient(System.Uri endpoint, Azure.AI.MetricsAdvisor.MetricsAdvisorKeyCredential credential, Azure.AI.MetricsAdvisor.MetricsAdvisorClientsOptions options) { }
public virtual Azure.Response<Azure.AI.MetricsAdvisor.Models.AnomalyAlertConfiguration> CreateAnomalyAlertConfiguration(Azure.AI.MetricsAdvisor.Models.AnomalyAlertConfiguration alertConfiguration, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual System.Threading.Tasks.Task<Azure.Response<Azure.AI.MetricsAdvisor.Models.AnomalyAlertConfiguration>> CreateAnomalyAlertConfigurationAsync(Azure.AI.MetricsAdvisor.Models.AnomalyAlertConfiguration alertConfiguration, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Response<Azure.AI.MetricsAdvisor.Models.DataFeed> CreateDataFeed(Azure.AI.MetricsAdvisor.Models.DataFeed dataFeed, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
Expand Down Expand Up @@ -106,15 +106,6 @@ public MetricsAdvisorAdministrationClient(System.Uri endpoint, Azure.AI.MetricsA
public virtual Azure.Response UpdateMetricAnomalyDetectionConfiguration(string detectionConfigurationId, Azure.AI.MetricsAdvisor.Models.AnomalyDetectionConfiguration detectionConfiguration, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual System.Threading.Tasks.Task<Azure.Response> UpdateMetricAnomalyDetectionConfigurationAsync(string detectionConfigurationId, Azure.AI.MetricsAdvisor.Models.AnomalyDetectionConfiguration detectionConfiguration, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
}
public partial class MetricsAdvisorAdministrationClientOptions : Azure.Core.ClientOptions
{
public MetricsAdvisorAdministrationClientOptions(Azure.AI.MetricsAdvisor.Administration.MetricsAdvisorAdministrationClientOptions.ServiceVersion version = Azure.AI.MetricsAdvisor.Administration.MetricsAdvisorAdministrationClientOptions.ServiceVersion.V1_0) { }
public Azure.AI.MetricsAdvisor.Administration.MetricsAdvisorAdministrationClientOptions.ServiceVersion Version { get { throw null; } }
public enum ServiceVersion
{
V1_0 = 1,
}
}
}
namespace Azure.AI.MetricsAdvisor.Models
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
<AssemblyTitle>Microsoft Azure.AI.MetricsAdvisor client library</AssemblyTitle>
<Version>1.0.0-beta.2</Version>
<TargetFrameworks>$(RequiredTargetFrameworks)</TargetFrameworks>
<NoWarn>
$(NoWarn);
AZC0007; <!-- https://github.com/Azure/azure-sdk-for-net/issues/16435 -->
</NoWarn>
Comment on lines +6 to +9
Copy link
Member Author

Choose a reason for hiding this comment

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

Blocked by Azure/azure-sdk-tools#127 (comment).

I tried suppressing the warning locally, but it still fails when exporting the API.

</PropertyGroup>

<!-- Shared source from Azure.Core -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ public MetricsAdvisorAdministrationClient(Uri endpoint, MetricsAdvisorKeyCredent
/// <param name="credential">A credential used to authenticate to the service.</param>
/// <param name="options">A set of options to apply when configuring the client.</param>
/// <exception cref="ArgumentNullException"><paramref name="endpoint"/> or <paramref name="credential"/> is null.</exception>
public MetricsAdvisorAdministrationClient(Uri endpoint, MetricsAdvisorKeyCredential credential, MetricsAdvisorClientOptions options)
public MetricsAdvisorAdministrationClient(Uri endpoint, MetricsAdvisorKeyCredential credential, MetricsAdvisorClientsOptions options)
{
Argument.AssertNotNull(endpoint, nameof(endpoint));
Argument.AssertNotNull(credential, nameof(credential));

options ??= new MetricsAdvisorClientOptions();
options ??= new MetricsAdvisorClientsOptions();

_clientDiagnostics = new ClientDiagnostics(options);
HttpPipeline pipeline = HttpPipelineBuilder.Build(options, new MetricsAdvisorKeyCredentialPolicy(credential));
Expand All @@ -71,12 +71,12 @@ internal MetricsAdvisorAdministrationClient(Uri endpoint, TokenCredential creden
/// <param name="credential">A credential used to authenticate to the service.</param>
/// <param name="options">A set of options to apply when configuring the client.</param>
/// <exception cref="ArgumentNullException"><paramref name="endpoint"/> or <paramref name="credential"/> is null.</exception>
internal MetricsAdvisorAdministrationClient(Uri endpoint, TokenCredential credential, MetricsAdvisorClientOptions options)
internal MetricsAdvisorAdministrationClient(Uri endpoint, TokenCredential credential, MetricsAdvisorClientsOptions options)
{
Argument.AssertNotNull(endpoint, nameof(endpoint));
Argument.AssertNotNull(credential, nameof(credential));

options ??= new MetricsAdvisorClientOptions();
options ??= new MetricsAdvisorClientsOptions();

_clientDiagnostics = new ClientDiagnostics(options);
HttpPipeline pipeline = HttpPipelineBuilder.Build(options, new BearerTokenAuthenticationPolicy(credential, Constants.DefaultCognitiveScope));
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ public MetricsAdvisorClient(Uri endpoint, MetricsAdvisorKeyCredential credential
/// <param name="credential">A credential used to authenticate to the service.</param>
/// <param name="options">A set of options to apply when configuring the client.</param>
/// <exception cref="ArgumentNullException"><paramref name="endpoint"/> or <paramref name="credential"/> is null.</exception>
public MetricsAdvisorClient(Uri endpoint, MetricsAdvisorKeyCredential credential, MetricsAdvisorClientOptions options)
public MetricsAdvisorClient(Uri endpoint, MetricsAdvisorKeyCredential credential, MetricsAdvisorClientsOptions options)
{
Argument.AssertNotNull(endpoint, nameof(endpoint));
Argument.AssertNotNull(credential, nameof(credential));

options ??= new MetricsAdvisorClientOptions();
options ??= new MetricsAdvisorClientsOptions();

_clientDiagnostics = new ClientDiagnostics(options);
HttpPipeline pipeline = HttpPipelineBuilder.Build(options, new MetricsAdvisorKeyCredentialPolicy(credential));
Expand All @@ -73,12 +73,12 @@ internal MetricsAdvisorClient(Uri endpoint, TokenCredential credential)
/// <param name="credential">A credential used to authenticate to the service.</param>
/// <param name="options">A set of options to apply when configuring the client.</param>
/// <exception cref="ArgumentNullException"><paramref name="endpoint"/> or <paramref name="credential"/> is null.</exception>
internal MetricsAdvisorClient(Uri endpoint, TokenCredential credential, MetricsAdvisorClientOptions options)
internal MetricsAdvisorClient(Uri endpoint, TokenCredential credential, MetricsAdvisorClientsOptions options)
{
Argument.AssertNotNull(endpoint, nameof(endpoint));
Argument.AssertNotNull(credential, nameof(credential));

options ??= new MetricsAdvisorClientOptions();
options ??= new MetricsAdvisorClientsOptions();

_clientDiagnostics = new ClientDiagnostics(options);
HttpPipeline pipeline = HttpPipelineBuilder.Build(options, new BearerTokenAuthenticationPolicy(credential, Constants.DefaultCognitiveScope));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ namespace Azure.AI.MetricsAdvisor
/// The set of options that can be specified when creating a <see cref="MetricsAdvisorClient" />
/// to configure its behavior.
/// </summary>
public class MetricsAdvisorClientOptions : ClientOptions
public class MetricsAdvisorClientsOptions : ClientOptions
{
private const ServiceVersion LatestVersion = ServiceVersion.V1_0;

/// <summary>
/// Initializes a new instance of the <see cref="MetricsAdvisorClientOptions"/> class.
/// Initializes a new instance of the <see cref="MetricsAdvisorClientsOptions"/> class.
/// </summary>
/// <param name="version">The version of the service to send requests to.</param>
/// <exception cref="ArgumentException"><paramref name="version"/> is <c>default</c>.</exception>
public MetricsAdvisorClientOptions(ServiceVersion version = LatestVersion)
public MetricsAdvisorClientsOptions(ServiceVersion version = LatestVersion)
{
if (version == default)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,23 +45,23 @@ public MetricsAdvisorAdministrationClient GetMetricsAdvisorAdministrationClientA
return InstrumentClient(new MetricsAdvisorAdministrationClient(
new Uri(TestEnvironment.MetricsAdvisorUri),
TestEnvironment.Credential,
InstrumentClientOptions(new MetricsAdvisorClientOptions())));
InstrumentClientOptions(new MetricsAdvisorClientsOptions())));
}

public MetricsAdvisorAdministrationClient GetMetricsAdvisorAdministrationClient()
{
return InstrumentClient(new MetricsAdvisorAdministrationClient(
new Uri(TestEnvironment.MetricsAdvisorUri),
new MetricsAdvisorKeyCredential(TestEnvironment.MetricsAdvisorSubscriptionKey, TestEnvironment.MetricsAdvisorApiKey),
InstrumentClientOptions(new MetricsAdvisorClientOptions())));
InstrumentClientOptions(new MetricsAdvisorClientsOptions())));
}

public MetricsAdvisorClient GetMetricsAdvisorClient()
{
return InstrumentClient(new MetricsAdvisorClient(
new Uri(TestEnvironment.MetricsAdvisorUri),
new MetricsAdvisorKeyCredential(TestEnvironment.MetricsAdvisorSubscriptionKey, TestEnvironment.MetricsAdvisorApiKey),
InstrumentClientOptions(new MetricsAdvisorClientOptions())));
InstrumentClientOptions(new MetricsAdvisorClientsOptions())));
}

internal static async Task<DataFeed> GetFirstDataFeed(MetricsAdvisorAdministrationClient adminClient)
Expand Down