-
Notifications
You must be signed in to change notification settings - Fork 370
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
Allow custom ReactPHP middlewares #439
Comments
Yeah custom middlewares would be good. However, we have already a ppm.json, which is basically the same as all CLI options. See https://github.com/php-pm/php-pm/blob/master/src/Commands/ConfigTrait.php#L82 |
@marcj my apologies then, hadn't even seen it 🤦♂️ An array of middlewares could be easily configured then |
I'm not sure if its PHP-PM's job to do this. AFAIK its not meant to replace a real web server, only to do what FPM used to do. |
@dnna yes, I agree, I don't expect PHP-PM to handle SSL termination, for examples. Maybe the gzip example was a bad one, there could be other useful middlewares. For example, there's a Static files middleware that we could reuse instead of having our own. |
Maybe this would be a good opportunity to approach this for 2.1 or 2.0? Do we agree that middlewares should be handled by the slaves, not the master? |
@andig It could also work on the master, yeah, might be easier maybe and middlewares like the static files one probably make more sense in master? |
mmm, ammending my own comment, we only start the reactphp HttpServer on the slaves, master just opens the socket and distributes the requests load among the slaves, never really touches the Http layer. Therefore, middlewares should go on the slaves if we want to keep the current architecture. |
I think the current architecture makes sense - IMO the master shouldn't do much more than distribute requests and ensure that the slaves are running. This way the system can scale by simply adding more workers, who are responsible for doing the actual work. Most of the middlewares I see on the page make sense to run on the slave, except the websocket one which would probably be a big milestone by itself. |
Yep. Also, things like Keep Alive or Http2 that the ReactPHP guys were working on (reactphp/http#39 reactphp/http#99) would probably mean the http server needs to be started on master too and the slaves are just piping back and forth a simple request/response |
Closing here in alignment with discussion in |
Sorry but I don't get it, what does the amphp discussion have to do with this? I'm happy to work on this feature (eventually) |
Absolutely nothing :O. Didn't realize that your PR was in httpkernel, sorry. |
hahaha gotcha, still, i don't think the 2 features were related, no? |
My thinking was that- if we were using Middleware's interface from PSR15 in php-pm/php-pm-httpkernel#146 then that would already be a first step into customer middlewares, so they are at least related or one is a precondition for the other? |
For example, right now we can't do Gzip compression directly from PHP-PM, we need a web server to do that for us. Not sure how could we implement this. At some point I feel we should start reading a php-pm.config.json file instead of having the huge amount of CLI options we have right now.
The text was updated successfully, but these errors were encountered: