-
Notifications
You must be signed in to change notification settings - Fork 498
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
Change Feed Processor: Adds support for Graph API accounts #2491
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ealsur
requested review from
FabianMeiswinkel,
j82w,
khdang,
kirankumarkolli,
kirillg,
neildsh and
sboshra
as code owners
May 21, 2021 19:46
j82w
reviewed
May 24, 2021
Microsoft.Azure.Cosmos/src/ChangeFeedProcessor/LeaseManagement/DocumentServiceLease.cs
Outdated
Show resolved
Hide resolved
...re.Cosmos/src/ChangeFeedProcessor/LeaseManagement/DocumentServiceLeaseStoreManagerBuilder.cs
Show resolved
Hide resolved
...os/src/ChangeFeedProcessor/LeaseManagement/PartitionedByIdCollectionRequestOptionsFactory.cs
Show resolved
Hide resolved
j82w
reviewed
May 24, 2021
...soft.Azure.Cosmos/src/ChangeFeedProcessor/LeaseManagement/DocumentServiceLeaseStoreCosmos.cs
Show resolved
Hide resolved
j82w
approved these changes
May 24, 2021
CPrashanth
approved these changes
May 26, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Graph API accounts are allowed to perform operations through the SQL API and as such, can also read the Change Feed.
Change Feed Processor uses a Lease Container that has, by default, a Partition Key Definition of
/id
. But/id
is not allowed in Graph API accounts, so Graph API users could not leverage Change Feed Processor even though they could do any SQL API operation.Azure/azure-documentdb-changefeedprocessor-dotnet#158 added Graph API support to the V2 Change Feed Processor.
This PR aims to port the same capability to V3 SDK to allow for users to migrate from V2 to V3 without any blocking issues.
The PR does not modify the public surface but just enables users to use Lease Containers that are partitioned by
/id
(current behavior) or partitioned by/partitionKey
(valid in Graph API accounts). The internal implementations will detect the path and generate the extra attribute on the leases in case it is needed.Strategic value
Since we are also migrating Azure Functions to V3 SDK (Azure/azure-webjobs-sdk-extensions#717) this will also mean that it would enable Azure Functions Triggers to be used on Graph API accounts, which was not previously possible.
Key changes
The PR introduces a new implementation of
RequestOptionsFactory
, so based on the Lease Container partition key definition, the CFP builder uses a different Factory implementation. The currentPartitionedByIdCollectionRequestOptionsFactory
or the newPartitionedByPartitionKeyCollectionRequestOptionsFactory
and the difference is one uses the value of theid
asPartitionKey
and the other, the value of thepartitionKey
, and one does not add thePartitionKey
property to lease documents (default), while the other does.Misc
Besides adding new sets of tests to cover the new behavior, the PR adds new tests for pre-existing APIs that had no test coverage and improves test run time (decreasing Emulator tests time) by using ManualResetEvent instead of waiting pre-defined amount of time on CFP tests.
Type of change
Closing issues
Closes #2291