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

Cosmos: Dispose queries/results if releasing against Cosmos SDK V3 instead of V4 #225

Closed
bartelink opened this issue Jun 18, 2020 · 2 comments · Fixed by #278
Closed

Cosmos: Dispose queries/results if releasing against Cosmos SDK V3 instead of V4 #225

bartelink opened this issue Jun 18, 2020 · 2 comments · Fixed by #278
Labels
Milestone

Comments

@bartelink
Copy link
Collaborator

NOTE This does not apply to the present 2.x release series

The Microsoft.Azure.Cosmos APIset introduces IDisposable on queries and responses - the current master/main branch uses this 'V3' API and hence should be updated to ensure memory usage is correct and/or optimal.

I believe the Azure.Cosmos (aka V4) SDK makes this moot by exposing the features as IAsyncEnumerable. Thus, assuming we don't release an impl targeting V3 and instead leapfrog from SDK v2 to V4 via #197 we can avoid needing any code changes.

@bartelink bartelink added the bug label Jun 18, 2020
@bartelink bartelink added this to the 3.0 milestone Jun 18, 2020
@kirankumarkolli
Copy link

@bartelink IAsyncEnumerable is IAsyncDisposable.

Usage pattern (await foreach) disposes underlying, is a convenience. Outside the that usage pattern its ideally expected to be disposed explictly.

@bartelink
Copy link
Collaborator Author

bartelink commented Jun 18, 2020

Thanks for clarifying @kirankumarkolli

The Equinox impl against V4 [preview3] in #197 already uses the higher level IAsyncEnumerable interface. Assuming the next V4 preview (with RU consumption fixes in 3.9.0 included too), this issue will thus become moot (i.e. there wont be a non-alpha release of Equinox.Cosmos targeting Microsoft.Azure.Cosmos and/or working with the lower level APIs).

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

Successfully merging a pull request may close this issue.

2 participants