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

Add StreamingRequestMiddleware to stream incoming requests, mark StreamingServer as internal #367

Merged
merged 3 commits into from
Jul 7, 2020

Conversation

clue
Copy link
Member

@clue clue commented Jul 6, 2020

This changeset adds a new StreamingRequestMiddleware to stream incoming requests and marks the existing StreamingServer as internal:

// unchanged: Server class is unchanged
$server = new React\Http\Server($handler);

// old: advanced StreamingServer is now internal only
$server = new React\Http\StreamingServer($handler);

// new: use StreamingRequestMiddleware instead of StreamingServer
$server = new React\Http\Server(array(
     new React\Http\Middleware\StreamingRequestMiddleware(),
     $handler
));

I've tried to keep the changeset as minimal as possible and most changes are in fact documentation only. See also each individual commit. Among others, this allows us to streamline the documentation and will allow us to improve default buffering behavior as discussed in #365/#360 and others. This will be filed as a follow-up PR once this PR is in.

Resolves #343

@clue clue added this to the v1.0.0 milestone Jul 6, 2020
@clue clue requested review from WyriHaximus and jsor July 6, 2020 20:21
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
clue added 2 commits July 7, 2020 08:54
This middleware can be used to process incoming requests with a
streaming request body (without buffering).

This will replace the existing `StreamingServer` class.
@clue
Copy link
Member Author

clue commented Jul 7, 2020

@jsor Thanks for spotting, updated :shipit:

@jsor jsor merged commit e4a81c1 into reactphp:master Jul 7, 2020
@clue clue deleted the streaming branch July 7, 2020 14:23
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.

Replace advanced StreamingServer with StreamingRequestMiddleware
3 participants