-
Notifications
You must be signed in to change notification settings - Fork 105
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
RecordsSubscribe #667
RecordsSubscribe #667
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
a482c6c
to
5ef8e8e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #667 +/- ##
==========================================
+ Coverage 98.86% 98.90% +0.03%
==========================================
Files 75 77 +2
Lines 9678 10015 +337
Branches 1423 1468 +45
==========================================
+ Hits 9568 9905 +337
Misses 102 102
Partials 8 8 ☔ View full report in Codecov by Sentry. |
8280c05
to
5b6d80f
Compare
…thorization property to prevent messageCid failure
06babf3
to
20fbcb4
Compare
@thehenrytsai I've updated the PR based on comments and discussions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
This PR builds on top of the existing
EventStream
and adds a subscription specifically for theRecords
interface which follows similar rules as those forRecordsQuery
.Currently the subscription does not evict or re-authorize subscriptions after the initial processing of the message. So if a revocation of permissions happens after the subscription is live, the user will continue getting notified until they close the connection.
This will be addressed in a separate PR to keep things more concise and easier to follow. #668