PeerSubscribers now specify what messages they are interested in #1137
Conversation
|
@gsalgado I still want to make some changes to the API so hold off on reviewing for now. Sorry for the pre-mature request. |
pipermerriam
force-pushed
the
piper/filters-for-peer-pool-subscribers
branch
from
2ce6ef8
to
edcb4d2
Compare
|
ok, ready for review, however, some brief testing shows this needs a bit more attention. |
pipermerriam
added
PR state: needs review
comp: p2p
type: feature
prio: P4 nice to have
effort: E1 hours
and removed
PR state: WIP
labels
pipermerriam
force-pushed
the
piper/filters-for-peer-pool-subscribers
branch
from
4a6cf77
to
355227d
Compare
|
Two things
Will investigate further tomorrow but wanted to make note of it here. |
| command which is not in this set will not be passed to this subscriber. | ||
|
|
||
| The base command class `p2p.protocol.Command` can be used to enable | ||
| **all** command types. |
There was a problem hiding this comment.
It might be worth mentioning that only sub-proto messages are delivered this way -- the base protocol ones are handled exclusively at the peer level
p2p/peer.py
Outdated
| peer, cmd, _ = msg | ||
|
|
||
| if not self.is_subscription_command(type(cmd)): | ||
| if hasattr(self, 'logger'): |
There was a problem hiding this comment.
Which subscribers don't provide a logger? Should we change them so they do?
There was a problem hiding this comment.
This is actually for the tests. The PeerSubscriber itself doesn't provide a logger. I'll remove this and change the ones in the test to have loggers.
What was wrong?
fixes #1055
How was it fixed?
All
PeerSubscriberclasses must now implement asubscription_msg_typeswhich defines which types of messages it will add to it's queue. Any message not the specified types is discarded. Including the basep2p.protocol.Commandin the list results in all messages being included in the queue.Cute Animal Picture