[Preview] Change Feed Processor: Refactors manual checkpoint API

[ealsur]

Based on the feedback on #2331 on the preview API, it would be better for the Manual Checkpoint API to be slightly refactored.

Current API is:

// Adds support for manual checkpoint on the typed API
public delegate Task ChangeFeedHandlerWithManualCheckpoint<T>(
	ChangeFeedProcessorContext context,
	IReadOnlyCollection<T> changes,
	Func<Task<(bool isSuccess, Exception error)>> tryCheckpointAsync,
	CancellationToken cancellationToken);

// Adds supports for manual checkpointing with stream handling
public delegate Task ChangeFeedStreamHandlerWithManualCheckpoint(
	ChangeFeedProcessorContext context,
	Stream changes,
	Func<Task<(bool isSuccess, Exception error)>> tryCheckpointAsync,
	CancellationToken cancellationToken);

Proposed API:

// Adds support for manual checkpoint on the typed API
public delegate Task ChangeFeedHandlerWithManualCheckpoint<T>(
	ChangeFeedProcessorContext context,
	IReadOnlyCollection<T> changes,
	Func<Task> checkpointAsync,
	CancellationToken cancellationToken);

// Adds supports for manual checkpointing with stream handling
public delegate Task ChangeFeedStreamHandlerWithManualCheckpoint(
	ChangeFeedProcessorContext context,
	Stream changes,
	Func<Task> checkpointAsync,
	CancellationToken cancellationToken);

The change is on the checkpoint delegate.

The reasoning behind the change is that Task already exposes an exception if it fails. So the Try signature returning a bool and Exception is ambiguous because the Task itself could also capture an exception.

[github-actions]

Please follow the required format: "[Internal] Category: (Adds|Fixes|Refactors|Removes) Description"

Internal should be used for PRs that have no customer impact. This flag is used to help generate the changelog to know which PRs should be included. Examples:
Diagnostics: Adds GetElapsedClientLatency to CosmosDiagnostics
PartitionKey: Fixes null reference when using default(PartitionKey)
[v4] Client Encryption: Refactors code to external project
[Internal] Query: Adds code generator for CosmosNumbers for easy additions in the future.

All good!

[j82w]

@kirankumarkolli and @FabianMeiswinkel please take a look at the API changes

[FabianMeiswinkel]

LGTM except for NITs where try/catch could be removed completely

[kirankumarkolli]

@ealsur , @j82w is it possible to cover these are breaking changes explicitly in preview release notes?

[j82w]

@ealsur , @j82w is it possible to cover these are breaking changes explicitly in preview release notes?

We can do a breaking changes section in the preview notes

[j82w]

Should this be something a little more obvious like changesPayload?

[ealsur]

Well, in this case, it is actually the list (IReadOnlyCollection<T>) of changes that happened. Those changes in the collection could be inserts or updates, but they are still changes. Not sure how the word payload makes it more clear?

[ealsur]

Also, the current API that is GA uses changes as name, so I rather keep the same as we have no feedback that indicates the name brings confusion?

[j82w]

Then it's fine. It just seemed a little off that a stream would be called changes.