-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Allowing customers to wrap CosmosAsyncContainer #43724
base: main
Are you sure you want to change the base?
Allowing customers to wrap CosmosAsyncContainer #43724
Conversation
API change check APIView has identified API level changes in this PR and created following API reviews. |
This is a good PR to open up the SDK a lot more in an easy to understand way.
|
Regarding #1 - the public API for readAllItems is meant to read all documents of a logical partition - this one can also be extended - azure-sdk-for-java/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/CosmosAsyncContainer.java Lines 1578 to 1674 in e948b62
Regarding #2 - ACK - this is true and it is (an intentional) limitation of CosmosPagedFlux - let's go through the concrete use cases to see whether/how we can unblock them by allowing public CosmosPagedFlux overloads or factory methods Regarding #3 - Correct - pipelining would be needed in this case - but I don't quite see why that is a problem. |
on #1 I aggree - not an issue |
Description
This PR adds a protected ctor for
CosmosAsyncContainer
to allow extending Container - for example to add custom diagnostics, error handling, retry policies or addiitonal validation - like disallowing certain query functionality etc.The
CosmosAsyncContainer
has several non-final methods already - but there was no public/protected ctor - this was intentional, because the business logic in theCosmosAsyncContainer
really does not allow for artificial extensions - like it won't make much sense to try to inject a different store than Cosmso DB because the logc for metrics, diagnostics and even the PagedIterator returned from query etc. is intentionally tight to Cosmos DB.There have been a few customer reports thought for limited extensibility - for testing mocking, but also to add some shared custom diagnostics and some validations to prevent application teams to violate against best practices (preventing cross-partition queries, disabling scan in query etc.)
So, this PR opens the extensibility in CosmosAsyncContainer a bit more - but since the only protected ctor still requires a CosmosAsnycContainer for the actual implementations it still only allows extensions to wrap around the original Container implementation. This PR is intentionally not trying to extend extensibility to support generic stores etc.
There are two options on how to inject custom containers.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines