-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Fixed composer resources between web/cli #23213
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
Conversation
Well, if we have some configuration set based on the presence of an extension (or the existence of a class provided by an extension), this would prevent purging the cache when needed |
I know, but I also consider this an edge-case. When you enable a module, it makes sense to use a clear cache. If your feature doesn't work after enabling the module, first thing you should do: clear your cache. Perhaps this check can be done elsewhere (WDT?) and show that the modules have changed between 2 requests. |
but if the cache was created in the CLI assuming the extension was there, and there you PHP-FPM process does not have it, your project is broken too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's do that, was already the case in 3.2 and before
I don't have have PHP-FPM and my project is not broken in the web because it doesn't have PCNTL. In this case, not having PCNTL in the web, is by design. Another solution that I've discussed with colleagues here, is making it configurable via possibly a composer extra setting. |
@stof, are you aware this is broken now for every setup where the CLI and Web extensions differ? (for instance, not having an apachehandler in CLI makes sense, since you do not need it). The current behavior triggers a container rebuild for every request for no reason. |
I don't get why you say this is triggering a rebuild for every request. |
@nicolas-grekas That's what I expected, but is not the case, most likely because we run commands on the CLI during cache warmup. But even in this case, calling the console would again trigger a rebuild, switching back to the web, another one. |
@nicolas-grekas ah yes, what @iltar says. During a request when the cache needs a warmup we run a CLI command, which triggers this bug again and rebuilds the container again for CLI. So at the end of that request we are back in "CLI-config" so the next request will do the same rebuild. Hence why it happens with every request. |
Let's do that and maybe add another more fine grained resource in 3.4 if we want to - tracking only the actually used extensions. |
Thank you @iltar. |
This PR was merged into the 3.3 branch. Discussion ---------- Fixed composer resources between web/cli | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no (reverts one) | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23212 | License | MIT | Doc PR | ~ This is a possible fix for the flawed module check for the composer resource. As this is the easiest fix, I've created a PR ready to be merged. Commits ------- 9e04712 Fixed composer resources between web/cli
This is a possible fix for the flawed module check for the composer resource. As this is the easiest fix, I've created a PR ready to be merged.