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

[BT-5271-streams-api] Align the Streams API with Event Feeds terminology. #164

Merged
merged 6 commits into from
Nov 6, 2024

Conversation

findgriffin
Copy link
Contributor

Description

  1. User-facing Stream API now takes an EventSource, and StreamOptions instead of StreamRequest.
  2. Replace usages of toStream() with eventSource().
  3. Add a new constructor to PageIterator that makes it possible to initialize with an existing page.
  4. Ensure the statsCollector gets updated for feed page responses, (Leftover from PR [BT-5270-events] #162)

Still TODO are README updates, final checks of test coverage, bug bash? etc.

Motivation and context

It's worth aligning the user-facing APIs before we GA the driver.

How was the change tested?

The unit and integ tests are fairly comprehensive, I also discovered a few things while building a sample app (how to do deletes, and the PageIterator gap).

Screenshots (if appropriate):

Change types

    • Bug fix (non-breaking change that fixes an issue)
    • New feature (non-breaking change that adds functionality)
    • Breaking change (backwards-incompatible fix or feature)

Checklist:

    • My code follows the code style of this project.
    • My change requires a change to Fauna documentation.
    • My change requires a change to the README, and I have updated it accordingly.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@findgriffin findgriffin requested a review from a team November 1, 2024 22:18
Copy link
Collaborator

@pnwpedro pnwpedro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general, but one question related to stats collection.

@pnwpedro pnwpedro merged commit eccb34d into main Nov 6, 2024
1 check passed
@pnwpedro pnwpedro deleted the BT-5271-streams-api branch November 6, 2024 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants