Skip to content
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

ChangeFeed Pull model API #1686

Closed
j82w opened this issue Jul 6, 2020 · 2 comments
Closed

ChangeFeed Pull model API #1686

j82w opened this issue Jul 6, 2020 · 2 comments

Comments

@j82w
Copy link
Contributor

j82w commented Jul 6, 2020

This issue will be used to discuss the public API changes made in PR #1332.

1. This is the current model:

Pros:

  • FeedRange and continuation token are mutually exclusive avoiding runtime conflicts
  • Promoted argument follows current SDK model

Cons:

  • Start From Beginning is just setting a magic number like DateTime.MinValue
  • Easy for users to use wrong date time format. Example is using local time instead of UTC datetime.
  • Users can set continuation token and start time causing ambiguity or runtime exceptions.
FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                feedRange,
                requestOptions: new ChangeFeedRequestOptions()
                {
                    StartTime = DateTime.MinValue,
                });

FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                feedRange,
                requestOptions: new ChangeFeedRequestOptions()
                {
                    StartTime = DateTime.UtcNow,
                });

FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(continuationToken);

// Invalid scenario users can easily run into
FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                continuationToken,
                requestOptions: new ChangeFeedRequestOptions()
                {
                    StartTime = DateTime.UtcNow,
                });

2. This is the current proposed model in PR #1332. Reverted from a split regression.

Pros:

  • Avoid users setting mutually exclusive start fields like continuation token and start from
  • StartFrom Beginning is just not setting a start from and avoid setting to a magic number like DateTime.MinValue
  • Enforces that StartTime is always in correct UTC format.

Cons:

  • Allows users to set FeedRange and continuation token causing runtime exceptions on mismatch
  • Inconsistent with current SDK model: No promoted property so users have to find it in request options.
  • A learning curve for users to find static method
FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                requestOptions: new ChangeFeedRequestOptions()
                {
                    From = ChangeFeedRequestOptions.StartFrom.CreateFromNow(),
                    FeedRange = feedRange,
                });

FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                requestOptions: new ChangeFeedRequestOptions()
                {
                    From = ChangeFeedRequestOptions.StartFrom.CreateFromBeginning(),
                    FeedRange = feedRange,
                });

// User can hit runtime exception by using wrong feed range with the continuation token
FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                requestOptions: new ChangeFeedRequestOptions()
                {
                    From = ChangeFeedRequestOptions.StartFrom.CreateFromContinuation(continuationToken),
                     FeedRange = feedRange,
                });

3. Slight modification of number 2. Promote StartFrom and take in optional FeedRange

Pros:

  • Avoid users setting mutually exclusive fields like continuation token and start from
  • StartFrom Beginning is just not setting a start from and avoid setting to a magic number like DateTime.MinValue
  • Enforces that StartTime is always in correct UTC format.
  • StartFrom is it's own class so it can be reused if necessary in the future
  • StartFrom can be a required field on the method avoiding any user ambiguity

Cons:

  • A learning curve for users to find static method, but there are other types in the SDK that follow this model.
FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                StartFrom.CreateFromNow(feedRange),
                new ChangeFeedRequestOptions()
                {
                     maxItemCount = 10
                 });

FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                StartFrom.CreateFromBeginning(feedRange),
                new ChangeFeedRequestOptions()
                {
                     maxItemCount = 10
                 });

FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                StartFrom.CreateFromContinuation("something"),
                new ChangeFeedRequestOptions()
                {
                     maxItemCount = 10
                 });

4. Merge of option 1 & 2 to use overloads

Pros:

  • Avoid users setting mutually exclusive fields like continuation token and start from
  • StartFrom Beginning is just not setting a start from and avoid setting to a magic number like DateTime.MinValue
  • Enforces that StartTime is always in correct UTC format.
  • Easier to discover feedRange and continuation token option than Option 3

Cons:

  • A learning curve for users to find static method
FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                 feedRange,
                ChangeFeedRequestOptions.StartFrom.CreateFromNow());

FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                feedRange,
                ChangeFeedRequestOptions.StartFrom.CreateFromBeginning());

FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                continuationToken);

5. ChangeFeedRequestOption factory

Pros:

  • Avoid users setting mutually exclusive fields like continuation token and start from
  • StartFrom Beginning is just not setting a start from and avoid setting to a magic number like DateTime.MinValue
  • Enforces that StartTime is always in correct UTC format.
  • Easier to discover feedRange and continuation token option than Option 3

Cons:

  • A learning curve for users to find static method
// feed range is optional on all methods
FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                ChangeFeedRequestOptions.CreateFromNow(feedRange));

FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                ChangeFeedRequestOptions.CreateFromBeginning(feedRange));

FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                ChangeFeedRequestOptions.CreateFromDateTime(feedRange));

FeedIterator feedIterator = itemsCore.GetStandByFeedIterator(
                ChangeFeedRequestOptions.CreateFromContinuation(continuationToken));
@ealsur
Copy link
Member

ealsur commented Nov 17, 2020

Closing as the related PRs were closed

@ealsur ealsur closed this as completed Nov 17, 2020
@ghost
Copy link

ghost commented Dec 15, 2021

Closing due to in-activity, pease feel free to re-open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants