This repository has been archived by the owner on Jun 11, 2024. It is now read-only.
Implement maximum number of inbound peer connections - Closes #3720 #3779
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diego-G
force-pushed
the
3720-implement_incoming_peer_connection_limit
branch
3 times, most recently
from
b258733
to
fc87ab3
Compare
Verify if the maximum number of inbound peer connections is respected in a sufficiently large sample.
diego-G
force-pushed
the
3720-implement_incoming_peer_connection_limit
branch
from
fc87ab3
to
fbc941d
Compare
diego-G
force-pushed
the
3720-implement_incoming_peer_connection_limit
branch
from
f38d731
to
a57d70e
Compare
mitsuaki-u
suggested changes
Jun 5, 2019
mitsuaki-u
reviewed
Jun 5, 2019
diego-G
force-pushed
the
3720-implement_incoming_peer_connection_limit
branch
from
faa80c6
to
373faf8
Compare
mitsuaki-u
approved these changes
Jun 5, 2019
jondubois
suggested changes
Jun 6, 2019
| public getAllPeers(): ReadonlyArray<Peer> { | ||
| return [...this._peerMap.values()]; | ||
| public getAllPeers( | ||
| kind?: typeof OutboundPeer | typeof InboundPeer, |
There was a problem hiding this comment.
This should be a string enum 'outbound' or 'inbound' for consistency. Also this is confusing because in JavaScript typeof OutboundPeer would evaluate to 'function'.
There was a problem hiding this comment.
What was the conclusion about this one? Although in tyepescript, this seems to be valid
There was a problem hiding this comment.
We have no better option and it is valid Typescript-wise.
There was a problem hiding this comment.
Because this method is only used internally by the PeerPool and not exposed outside the P2P library, I was convinced that it's OK in this case.
jondubois
approved these changes
Jun 6, 2019
shuse2
suggested changes
Jun 6, 2019
shuse2
approved these changes
Jun 7, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
How to test it?
npm run test:integrationReview checklist