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

Sync read of request body throws on .netcore 3.0 #2614

Merged
merged 1 commit into from
Oct 7, 2019
Merged

Sync read of request body throws on .netcore 3.0 #2614

merged 1 commit into from
Oct 7, 2019

Conversation

ladeak
Copy link
Contributor

@ladeak ladeak commented Sep 27, 2019

Serializing http request and response bodies async from memorystream, to avoid .netcore3 sync IO restrictions.
This PR does not address similar issues in Twilio, WebEx and ApplicationInsights projects.

An alternative option would be to use System.Text.Json with superior performance (memory and CPU), but serialization settings would need to be regression tested + it would introduce a new dependency to this package.

@msftclas
Copy link

msftclas commented Sep 27, 2019

CLA assistant check
All CLA requirements met.

@ladeak
Copy link
Contributor Author

ladeak commented Sep 27, 2019

How can I trigger CI?

@cleemullins
Copy link
Contributor

This comment here should trigger the CI build.

@cleemullins
Copy link
Contributor

@gabog, when you get time could you look into this? Everything here looks reasonable, but there may be more to do.

@ladeak
Copy link
Contributor Author

ladeak commented Sep 27, 2019

@cleemullins I do not see the CI triggered

@ladeak
Copy link
Contributor Author

ladeak commented Oct 2, 2019

@cleemullins or @gabog any updates?

@cleemullins
Copy link
Contributor

@gabog or @garypretty could you review this when you get a moment?

Copy link
Contributor

@garypretty garypretty left a comment

Choose a reason for hiding this comment

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

LGTM - for reference, details of the changes to disable synchronous read / write of body can be found in the Microsoft docs for migrating from .NET Core 2.2 to 3.

@cleemullins
Copy link
Contributor

Gary has built / tested this, and run a full CI build against the code. This is good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants