Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
stof commentedJun 16, 2017
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 |
linaori commentedJun 16, 2017
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. |
stof commentedJun 16, 2017
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 |
nicolas-grekas left a comment
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
linaori commentedJun 16, 2017
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. |
yannickl88 commentedJun 16, 2017
@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 forevery request for no reason. |
nicolas-grekas commentedJun 16, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I don't get why you say this is triggering a rebuild forevery request. |
linaori commentedJun 16, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@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. |
yannickl88 commentedJun 16, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@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 containeragain 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. |
nicolas-grekas commentedJun 16, 2017
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. |
fabpot commentedJun 16, 2017
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.