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

LimitHandlers Middleware #218

Closed

Conversation

WyriHaximus
Copy link
Member

LimitHandlers Middleware split off from #213
Assumes to be used with #215
Is blocked by #216 for the readme (will create merge conflicts otherwise)

Not sure about name, LimitConcurrentHandlers might be a better name, thoughts?

$body->expects($this->once())->method('pause');
$body->expects($this->once())->method('resume');
$limitHandlers = new LimitHandlers(1);
$limitHandlers(new ServerRequest('GET', 'https://example.com/', [], $body), function () {});
Copy link
Member

Choose a reason for hiding this comment

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

[] -> array() for our old friend PHP 5.3

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed -.-

@jsor
Copy link
Member

jsor commented Sep 11, 2017

Not sure about name, LimitConcurrentHandlers might be a better name, thoughts?

If find the handlers part a little bit unclear. Brain dump:

  • LimitConcurrentRequests
  • LimitConcurrentConnections
  • LimitConcurrentClients

@WyriHaximus
Copy link
Member Author

Connections isn't an option as the connection already has been made and is active, just pauzed when in the queue, same goes for clients tbh.

@WyriHaximus
Copy link
Member Author

@jsor what do you think about: LimitConcurrentRequestHandlers, a bit of a mouth full though 🤐

@jsor
Copy link
Member

jsor commented Sep 12, 2017

@WyriHaximus That might work, yes. Maybe @clue has some more ideas.

@WyriHaximus
Copy link
Member Author

@jsor currently stuck on the fact that the request stream pausing doesn't work as intended. When it's paused it will miss out on contents of the request body as shown by the functional test I've added. Hope @clue has some ideas for that more then the name at the moment 😛

@WyriHaximus
Copy link
Member Author

Removed the mile stone because we hit a few roadblocks implementing this properly. Leaving this open for the time until we find a solution.

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

Successfully merging this pull request may close these issues.

None yet

2 participants