-
Notifications
You must be signed in to change notification settings - Fork 174
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
Break on empty events when loading from remote #2376
Conversation
WalkthroughOhayo, sensei! The changes involve a modification to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Provider
Client->>Provider: Request events with continuation token
Provider-->>Client: Return events and continuation token
alt Events returned
Client->>Client: Append events to collection
Client->>Provider: Request next events with token
else No events returned
Client->>Client: Break loop
end
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2376 +/- ##
==========================================
+ Coverage 67.72% 67.85% +0.12%
==========================================
Files 359 359
Lines 47034 46959 -75
==========================================
+ Hits 31854 31862 +8
+ Misses 15180 15097 -83 ☔ View full report in Codecov by Sentry. |
currently we only stop querying for events when the provider no longer returns a continuation token. this pr adds an extra safety net to stop immediately when there are no more events being returned.
though the spec mentions explicitly that the token
Should not appear if there are no more pages.
but it doesn't mention exactly how the token should behave with pending block as the list of events in the pending block could still grow.there could be a case where someone calls
getEvents
with the theto
block setsto=pending
. if they already reach the end of the pending block during the first call, yet the block could still emit new events. but if the returned continuation token is nil, then if the user wants to continue from the last events in the first call, they couldn't and had to start from the beginning again.ref: #2375
Summary by CodeRabbit