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] Make collections returned by service methods pageables #16049

Merged
merged 4 commits into from
Oct 21, 2020

Conversation

kinelski
Copy link
Member

@kinelski kinelski commented Oct 16, 2020

Updated methods that return IReadOnlyList to return pageables. Getting this PR out first so the samples PR doesn't get too big.

Fixes #15927.

@kinelski kinelski marked this pull request as ready for review October 16, 2020 22:11
@kinelski kinelski requested review from christothes and maririos and removed request for tg-msft and KrzysztofCwalina October 16, 2020 22:11
@kinelski kinelski self-assigned this Oct 16, 2020
@kinelski kinelski added Client This issue points to a problem in the data-plane of the library. Cognitive - Metrics Advisor labels Oct 16, 2020

List<AnomalyAlertConfiguration> getAlertConfigs = new List<AnomalyAlertConfiguration>();

await foreach (var config in adminClient.GetAnomalyAlertConfigurationsAsync(createdAnomalyDetectionConfiguration.Id))
Copy link
Member

Choose a reason for hiding this comment

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

Me learning, is this the only way to get the list of items after they are in the AsyncPageable type?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the only "natural" approach, as far as I know. AsyncPageable implements the IAsyncEnumerable interface, and that's why it's accessed like this (more info about them here).

I think you could do something like this as well:

var enumerator = adminClient.GetSomethingAsync(...).GetAsyncEnumerator();

while (await enumerator.MoveNextAsync())
{
  var element = enumerator.Current;
  // Do something with <element>
}

That's probably what happens under the hood. I don't see a good reason for preferring this approach, though (maybe if you're using an older version of C# and can't use await foreach).

Copy link
Member

Choose a reason for hiding this comment

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

I remembered I saw this somewhere once in appconfig. You could use ToEnumerableAsync.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only a convenience method in our test framework, though. Good to know.

I thought you were asking from a user's point of view.

Copy link
Member

@christothes christothes left a comment

Choose a reason for hiding this comment

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

Sorry, I forgot about this one!

@kinelski kinelski merged commit 485e510 into Azure:master Oct 21, 2020
@kinelski kinelski deleted the ma-pageable branch October 21, 2020 19:13
annelo-msft pushed a commit to annelo-msft/azure-sdk-for-net that referenced this pull request Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Cognitive - Metrics Advisor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MetricsAdvisor] Consider making collections returned by service methods pageables
3 participants