Closed CPrashanth closed 3 years ago
CFP uses /id
as lease store partition key path. In Graph accounts /id
is not allowed.
The only problem with this scenario is that the point 3:
create a sql collection and write data into it (Cosmosdb allows it)
Collections are not SQL, in a Graph account, all containers are Graphs, you cannot select a container not to be considered a Graph from the service perspective.
Cosmos DB allows Graph accounts to use the SQL API for querying purposes, but these are still Graphs and can contain Graph data.
We could add an optional configuration for Change Feed Processor to specify the Partition Key path of the lease store, but the only scenario would be for Graph users, not for SQL API users (which would see no point in this configuration). I'm a bit reluctant to add a configuration that won't benefit the main audience of this SDK.
But I'm open to discussion.
if you can do CRUD using Sql in a graph account, isn't it for all practical purpose from customer point of view a sql collection. shouldnt changefeed which is also part of the same Sql api be supported in the graph account. Why is changefeed alone special cased out?
2nd problem- how to write UT/CTs if a microservice targets scenario using graph api for one collection, sql api for another collection. we got to start emulator in gremlin mode for graph api, stop it n restart in sql mode. it gets messy to do this coordination using mstest especially if tests run in parallel targeting different collection.
only change required is to add a property name other than "id" to make changefeed api work even for gremlin account and this can be internally handled within library
Change Feed works, you can use the Change Feed pull model or change feed queries if you want and they will work.
The Change Feed Processor, which is a construct particularly made for SQL API accounts (and not the only way to consume the Change Feed), does not work because the lease container definition uses /id
which is valid in a SQL API account but invalid for Gremlin API accounts for some reason.
only change required is to add a property name other than "id" to make changefeed api work even for gremlin account and this can be internally handled within library
This is not trivial, the lease schema does not have any other property that has the same characteristics as the /id
that would ensure the same distribution.
The second problem is unrelated to this repository.
isnt the problem simple as adding another JsonProperty where LeaseId=>this.Id in the class and enable PartitionKey support on this LeaseId property name. I believe rest of the code can still operate out of the this.Id field?
Not saying the problem's complexity is extreme, but when modifying the public API of the SDK to add any configuration setting that would be exposed to all users, the use cases need to be understood, the API names should be clear enough so that a user on a SQL API account (which is the vast majority) understand that they do not need to change it for example.
I added the feature
tag to mark this is an ask for a feature that will get evaluated as there is time available, currently our feature planning is full for the upcoming months but if you want to send a PR with the proposal, we could evaluate it for sure.
Following up this request: we'll be porting the change that happened in the V2 CFP: https://github.com/Azure/azure-documentdb-changefeedprocessor-dotnet/pull/158
This is an issue even for V2 Changefeed library. scenario- 1) create a gremlin cosmosdb account 2) create a graph collection 3) create a sql collection and write data into it (Cosmosdb allows it) 4) setup changefeed for sql collection created in step 3
expected: changefeed to work actual: changefeed fails because changefeed collection partition key path is set as /id and gremlin cosmosdb account doesnt like it.
customer impact: creating another cosmosdb account just to host changefeed collection is not feasible. if graph collection and sql collection has to be separated then cosmosdb ui shouldnt allow creation in the first place. Also emulator doesnt support multiple cosmosdb accounts so changefeed is not testable in UT/CT it would be great if you can patch the fix into V2 Api as well