Skip to content

Removing CancelableAsyncEnumerable #32090

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 13, 2021
Merged

Removing CancelableAsyncEnumerable #32090

merged 2 commits into from
May 13, 2021

Conversation

ladeak
Copy link
Contributor

@ladeak ladeak commented Apr 23, 2021

Removing CancelableAsyncEnumerable
Using a while loop as discussed in a PR discussion previously: #6791 (comment)

Description

  • Using a while loop to iterate items
  • Testing cancellation during streaming

Addresses #7960

@ghost ghost added area-signalr Includes: SignalR clients and servers community-contribution Indicates that the PR has been added by a community member labels Apr 23, 2021
@BrennanConroy BrennanConroy self-assigned this Apr 23, 2021
@ladeak
Copy link
Contributor Author

ladeak commented Apr 27, 2021

@BrennanConroy and @halter73 may I get a review?

@ladeak ladeak marked this pull request as draft May 4, 2021 18:21
@ladeak ladeak force-pushed the ladeak_7960 branch 3 times, most recently from deadace to 1ce2bdd Compare May 5, 2021 21:50
@ladeak ladeak marked this pull request as ready for review May 6, 2021 18:14
@ladeak ladeak requested a review from halter73 May 6, 2021 18:14
@ladeak
Convert MakeEnumerable and MakeEnumerableChannel to making IAsyncEnum…
eae2eb2 
…erator

DefaultHubDispatcher iterates using the enumerator
Adding a tests
halter73
halter73 approved these changes May 12, 2021
View reviewed changes
Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

LGTM pending adding the /Shared/ .ConfigureAwait(false) back. Thanks!!

@ladeak
Copy link
Contributor Author

ladeak commented May 13, 2021

Thank you for the update. I am seeing that the Linux-ARM64 build is failing with

/home/vsts/work/1/s/eng/common/tools.sh: line 459:  2697 Segmentation fault      (core dumped) "$_InitializeBuildTool" "$@"
Build failed with exit code 139. Check errors above.

It seems this is some tooling issue. I would like to re-run builds (or at least that build) but I don't see a button for that. Is there a way to do that (other than pushing an update to the branch)?

BrennanConroy
BrennanConroy approved these changes May 13, 2021
View reviewed changes
Copy link
Member

@BrennanConroy BrennanConroy left a comment

Choose a reason for hiding this comment

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

Excellent! Thanks for doing this!

@BrennanConroy BrennanConroy merged commit ade3d6e into dotnet:main May 13, 2021
@ghost ghost added this to the 6.0-preview5 milestone May 13, 2021
@BrennanConroy BrennanConroy mentioned this pull request May 13, 2021
Closed
@BrennanConroy
Copy link
Member

Thanks again @ladeak !

@ladeak ladeak deleted the ladeak_7960 branch May 19, 2021 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@halter73 halter73 halter73 approved these changes

@BrennanConroy BrennanConroy BrennanConroy approved these changes

Assignees

@BrennanConroy BrennanConroy

Labels
area-signalr Includes: SignalR clients and servers community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
6.0-preview5
Development

Successfully merging this pull request may close these issues.
3 participants
@ladeak @BrennanConroy @halter73