feat: unify php_server logic - #2499
Conversation
09cd397 to
39db699
Compare
|
Some more small fixes by @nicolas-grekas are now merged in (see #2565) I also added some tweaks to make startups race-free.
|
| func (f FrankenPHPApp) CaddyModule() caddy.ModuleInfo { | ||
| func (*FrankenPHPApp) CaddyModule() caddy.ModuleInfo { | ||
| return caddy.ModuleInfo{ | ||
| ID: "frankenphp", | ||
| New: func() caddy.Module { return &f }, | ||
| New: func() caddy.Module { return &FrankenPHPApp{} }, | ||
| } | ||
| } |
There was a problem hiding this comment.
Makes much more sense to not return the same app instance. Probably was the root cause of many subtle bugs in tests
|
I'll probably merge this branch soon if that's fine with everyone involved . Otherwise we'll just end up stalling other PRs |
|
I'm trying to tag a release. I hope it will be good tomorrow. Please wait for the new release to be tagged before merging, so we'll have more time to test this when merged in main. |
|
Looking through global options again, it probably also makes sense to move server := NewServer(root string, opts... ServerOption )
func WithServerLogger(logger) ServerOption
func WithServerMaxRequests(num) ServerOption
... |
|
I agree, it feels like we'll add more options in the future |
|
Let's get this merged first 😆 . There are plenty other features waiting on this too. |
|
Added the server options just to not have BC breaks. Ready to merge whenever |
|
Will merge if there's no other blocker @dunglas. There's still some more cleanup to do, doesn't need to all be in this PR though. |
|
LGTM! |
Currently the concept of a
php_serveronly exists on the caddy side and not the FrankenPHP side.Lately we have been moving more and more in a direction of scoping requests or workers to specific
php_serverblocks.This PR is an attempt at refactoring the current
php_serverlogic so it is properly mirrored on the FrankenPHP side without BC breaks for library users (and to prevent future bugs like mentioned in #2487)