Skip to content
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

Basic working connection limiter #50

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Basic working connection limiter #50

wants to merge 2 commits into from

Conversation

Davidde94
Copy link
Contributor

Example of a (very) simple echo server that only allows n active connections at any one time.

@Davidde94 Davidde94 requested a review from weissi May 15, 2020 13:26
@Davidde94 Davidde94 marked this pull request as ready for review August 3, 2020 11:20
@Davidde94 Davidde94 requested review from glbrntt, Lukasa and PeterAdams-A and removed request for weissi August 3, 2020 11:20
@Davidde94
Copy link
Contributor Author

Made this a while ago, might as well get it reviewed or let it die. Original use case was to limit the number of IMAP connections.

@@ -0,0 +1,3 @@
# connection-limiter

A description of this package.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wanna throw together some actual text here?

@@ -0,0 +1,64 @@

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need the license header.

func channelRead(context: ChannelHandlerContext, data: NIOAny) {
let channel = self.unwrapInboundIn(data)
context.fireChannelRead(data)
self.currentConnections += 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is never decremented. That doesn't seem likely to be right.

channel.pipeline.addHandler(LimitHandler(connectionLimit: 1))
})
.serverChannelOption(ChannelOptions.maxMessagesPerRead, value: 1)
.serverChannelOption(ChannelOptions.backlog, value: 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting this to 1 is not something we'd recommend in a real app and it doesn't affect the correctness of the example, so let's remove this.


final class connection_limiterTests: XCTestCase {

}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May as well just remove the tests entirely.


import NIO

class LimitHandler: ChannelDuplexHandler {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these examples are often helpful for people who know they want to do something but don't quite know how: as such I think the documentation and comments here should be comprehensive.  

@Lukasa Lukasa changed the base branch from master to main September 24, 2020 14:36
@Lukasa
Copy link
Contributor

Lukasa commented Sep 24, 2020

Updated this PR to target main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants