-
Notifications
You must be signed in to change notification settings - Fork 102
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
EventsQuery by protocol #762
Conversation
a2ed885
to
babbd40
Compare
e07830b
to
159dfa4
Compare
159dfa4
to
18069cd
Compare
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.
You really did go all out at removing loads of tests!
Just a couple of comments on renaming/rewording.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #762 +/- ##
=======================================
Coverage 98.64% 98.64%
=======================================
Files 71 71
Lines 10893 10899 +6
Branches 1577 1564 -13
=======================================
+ Hits 10745 10751 +6
Misses 142 142
Partials 6 6 ☔ View full report in Codecov by Sentry. |
@thehenrytsai addressed the review comments, and also created and added a reference to this issue: |
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 is to implement a
protocol
filter for theEventsQuery
interface which accounts for receiving permission messages scoped to the filtered protocol via tag filters, as well as other messages directly indexed to the protocol.It also resolves this issue #663, which simplifies the type of filters
EventsQuery
implements to reduce complexity of which messages may or may not be returned with the filter.Since
EventsQuery
is being used for sync/selective sync, theprotocol
filter is most important, if further filters are needed down the line, we will implement them according to scenarios put in place so we can make sure the correct messages arrive with the query results.The Additional filters I left are for
interface
,method
andmessageTimestamp
which are compatible with all types of messages.NOTE: I cleaned up something I didn't like with
EventsQuery
from the prior PR instead of the message descriptor having an undefined filter, it is forced to have an empty filters array.NOTE2:
EventsSubscribe
does not yet handle filtering for protocols which include the permissions messages, that will be done in a separate PR.