-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Router] add support for other web servers in router:dump #2432
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
Comments
I looked at this recently but it doesn't seem possible in nginx to set environment variable per request for one call to the backend, so this may not be possible unless we hack it with custom headers or custom query args. |
It is possible by using http://nginx.org/ru/docs/http/ngx_http_headers_module.html (no English version available, try google translate it) to set headers per request. (With "if" constructions from ngx_http_rewrite_module ) For example:
In example we use ApacheUrlMatcher header patterns, however we can define out own in separate class for nginx. If no one deals with this issue yet, I can implement this in few days. |
After reading the English version [1] of above link, I don't think that this is what we are looking for. As what this module does is sets the new headers for a response, but we need to pass new environment variables to the php script. |
Ok. I believe that there is no way to pass ENV variables from nginx without writing separate module for it. However there is solution to write NginxUrlMatcher that would check SERVER variable. |
Closing this issue as there is apparently no viable option (yet) for nginx. |
It doesn't make sense to close this as a large majority. After debugging SF2 with Apache vs Nginx... Nginx / PHP-FPM blew it out of the water and am using it for production, however, our application has around 90 routes so far and seems like overkill to go through up to 90 regex if statements to match a given route... there's gotta be another solution? I'm not sure how much faster a route dump would be. |
@jstout24 first, it will probably not go through 90 regexes as the dumper applies some optimizations. And to dump the matching for Nginx, you would need to modify Nginx to allow passing ENV variables |
It's possible to set $_SERVER variables with the
The ApacheUrlMatcher already check the $_SERVER variables, so should work out of the box, the only missing part is the dumper. |
Is there any plan to re-look at this issue? I'm also considering nginx as well. |
As @madarco says |
It should be quite easy, starting from the Apache dumper, to do the same thing but with the nginx syntax. However, I've done some tests and the speed benefit is almost zero, since the php dumper is already optimized (it create nested IFs and the regexps have almost the same speed as in nginx). For the tests I've implemented the nginx rules by hand and used the ApacheUrlMatcher as the router, as in my previous example.. |
What is the benefit from dumping routes for the apache then? Apache doesn't seem to be faster then Nginx. |
The Apache dumper was written before the optimizations were done in the PHP dumper. I think it's safe to deprecated the Apache dumper now (will do that now). |
@fabpot have you benchmarked this new PHP matcher vs the Apache? |
I'm not using Apache anymore, but I'd be very interested in seeing the results of such a benchmark. |
I'll try to write version for nginx this weekend. So I and everybody else could repeat the benchmark and make sure PHPMatcher is good enough (or not :)). After that we will be able to decide whether we should go on. |
Well, given that the ApacheDumper (or a yet-to-be-implemented NginxDumper) cannot be used when using the new expression-based condition added in 2.4 (it cannot evaluate the expression), I don't think it is worth trying it |
@AlexeyKupershtokh see #9652 discussing the deprecation |
I agree with you. But it's interesting if there's any performance gain we could make use of at all. |
+1 We have a legacy application and a Symfony application running on the same domain. Would be nice if we could separate the requests to the apps by using Nginx. |
The dumper is deprecated so there won't be any code updates to it. |
most importantly nginx seems to be a popular choice
The text was updated successfully, but these errors were encountered: