EF team feedback on various APIs

[azure]

EF team feedback on various APIs

[AndriySvyryd]

@ealsur I see, then we would still need a way of running the custom handler before the default one or removing it.

[j82w]

1. What's the use case for multiple paths?
2. Currently there are several dependencies that use this, but an issue can be created to track it..
3. This is a good idea. Need to create an issue to track the feature request
4. The type is required to deserialize the response stream to the object.
5. I believe the rename will be the best option. We moved it to a parameter to make sure developers are aware of it, and for it to be easier to set.
6. There currently is a bug that is preventing most partition key types and that will be fixed in the next release. We can definitely add more documentation to clarify the requirements.
7. The CosmosConfiguration contains MaxRetryAttemptsOnThrottledRequests and MaxRetryWaitTimeOnThrottledRequests . Are there additional properties or settings that are needed?
8. An issue can be created for this.

[AndriySvyryd]

Add a constructor to PartitionKeyDefinition that accepts an IEnumerable of paths

1. What's the use case for multiple paths?

The benefit of the constructor is the ability to create CosmosContainerSettings in one expression:

var settings = new CosmosContainerSettings
{
    Id = parameters.ContainerId,
    PartitionKey = new PartitionKeyDefinition("/pk")
};

instead of

var pk = new PartitionKeyDefinition();
pk.Paths.Add("/pk");

var settings = new CosmosContainerSettings
{
    Id = parameters.ContainerId,
    PartitionKey = pk
};

Allow to provide a custom IDocumentClientRetryPolicy or provide a built in way of using exponential backoff.

7. The CosmosConfiguration contains MaxRetryAttemptsOnThrottledRequests and MaxRetryWaitTimeOnThrottledRequests . Are there additional properties or settings that are needed?

For bursty highly parallelized workloads a more aggressive delay strategy could be required:

1. Additional delay in the form of c^n where c is a user-specified value and n is the current number of failed retry attempts
2. Additional random delay up to the specified amount

[AndriySvyryd]

There doesn’t seem to be an advantage to have CosmosResponse.Resource be strongly typed, as the generic argument cannot always be inferred and frequently needs to be specified explicitly. Returning it as object makes using the methods like DeleteItemAsync<>() simpler as they won’t need the generic argument.

4. The type is required to deserialize the response stream to the object.

In the case of DeleteItemAsync<T> Resource is always null

In cases where it isn't you can make deserialization lazy by changing Resource property to a method - GetResource<T>()

[j82w]

1. There is a constructor overload that takes in the path.

var settings = new CosmosContainerSettings(
                id: "SampleContainer",
                partitionKeyPath: "/pk");

In the case of DeleteItemAsync Resource is always null

In cases where it isn't you can make deserialization lazy by changing Resource property to a method - GetResource()

4. The reason the typed is not lazy is we wanted avoid having everything implement IDisposable. The original response back from Cosmos contains a Stream that needs to be disposed. If we did a lazy deserialization then all the responses would need to wrapped in using statements in the scenario that the objects are never deserialized. This made basic applications look very complex and was very easy for developers to have memory leaks. If you need additional performance or want to do your own deserialization you can use the stream APIs.

For bursty highly parallelized workloads a more aggressive delay strategy could be required:

Additional delay in the form of c^n where c is a user-specified value and n is the current number of failed retry attempts
Additional random delay up to the specified amount

7. We are planning to design a new retry system based on the handler pipeline that would allow custom retry handlers. We're still in the process of planning so I'm not sure when this will be worked on.

[AndriySvyryd]

4. The reason the typed is not lazy is we wanted avoid having everything implement IDisposable. The original response back from Cosmos contains a Stream that needs to be disposed. If we did a lazy deserialization then all the responses would need to wrapped in using statements in the scenario that the objects are never deserialized. This made basic applications look very complex and was very easy for developers to have memory leaks. If you need additional performance or want to do your own deserialization you can use the stream APIs.

The stream APIs response needs to be disposed anyway. If you established that all responses need to be disposed it would actually make it less likely for a developer to forget to do it as it would be consistent across all of your API and shown in all samples.

[ealsur]

@AndriySvyryd @divega Re 7: You can apply your custom retry policy as a Handler, in our samples, there is one that covers using Polly: https://github.com/Azure/azure-cosmos-dotnet-v3/blob/master/Microsoft.Azure.Cosmos.Samples/CodeSamples/Handlers/ThrottlingHandler.cs

[AndriySvyryd]

@ealsur Thanks, would that replace the built-in retry policy or would it run on top of it?

[ealsur]

It would run on top of it. If the custom retry is covering a scenario the built-in retry handler already covers, then the default one will be applied first. V3 uses Retry handlers such as https://github.com/Azure/azure-cosmos-dotnet-v3/blob/master/Microsoft.Azure.Cosmos/src/Handler/RetryHandler.cs to apply the default retry policy. If no default retry handler exists to cover that case, then your custom handler would be the first responder.

[ealsur]

Or expose the default one retry options, that would work too, just in the V2 SDK.

[kirankumarkolli]

Please create specific issues for gaps if-any.