-
Notifications
You must be signed in to change notification settings - Fork 60
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
fix(p2p): fix EventChannel, add tests #293
Conversation
} | ||
|
||
impl<T> EventChannel<T> { | ||
pub fn new(capacity: usize, config: ChannelSinkConfig) -> (Self, mpsc::Receiver<T>) { |
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.
Since an EventChannel<T>
can only have the values of ChannelSinkConfig
this implementation is Ok, but I would suggest to leave room for extension: Reduce the responsibility of new
to one, create new functions like new_blocking(..)
, new_drop_latest(...)
, etc...
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.
I personally favor having a single new method with an enum, over having a separate new
method for each variant. Especially since the rest of the parameters (in this case only capacity
) is the same for all variants, hence all 3 methods would have the same signature.
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.
May I also ask you to describe the bug in a few sentences, that has been fixed? Thank you :)
The bug was caused by a fundamental misunderstanding on my side of what |
15a7b29
to
b64e615
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.
❤️
Description of change
Refactor
Sink
andStream
implementation forEventChannel
.The
Sink
implementation allows to write a message to the channel (blocking or non-blocking).In case of an non-blocking
Sink
, theStream
implementation has to be continuously polled. The polling drives the sending of messages once the inner channel has the capacity to receive it (in theSwarmTask
, this polling is done in the central loop inStream::run
).The
ChannelSinkConfig
allows to configure the channels behaviour:ChannelSinkConfig::Block
:Sink
implementation delegates every call to the innermpsc::Sender
EventChannel::send
returnsEventChannel::send
blocks until the message can be sendStream
implementation is not relevant.ChannelSinkConfig::DropLatest | ChannelSinkConfig::BufferLatest
:<EventChannel as Stream>
flushes the messageChannelSinkConfig::BufferLatest
<EventChannel as Stream>
writes from buffer to inner channel once there is capacityChannelSinkConfig::DropLatest
: drop messageType of change
Choose a type of change, and delete any options that are not relevant.
How the change has been tested
Added
event_channel::test
module to test everyChannelSinkConfig
.Change checklist