Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
add bulkupdateasync #2655
add bulkupdateasync #2655
Changes from all commits
c0741e4
c7738a7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does request message get disposed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we dispose the request object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is it getting disposed?
I see that we're disposing
perReq
at some point, but looking at the source code ofPerRequest.Dispose()
, it doesn't seem to be disposing it'sPerRequest.Request
object. So it's still not clear to me at what point we're disposingthis.bulkUpdateRequestMessage
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perReq
is a wrapper of the request object. When it is disposed it disposes its contents. We assignthis.bulkUpdateRequestMessage
topereq.Request
. Whenpereq.Dispose()
is called,pereq.Request
is set tonull
. I don't think we can callDispose()
on theperRequest.Request
object.ODataRequestMessageWrapper
does not implement theIDisposable
interface.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. It's strange that it doesn't implement
IDisposable
, seems like design flaw. I wonder whether we may have any resource leaks since at least the underlyingHttpClientRequestMessage
is disposable.