Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
fabpot merged 1 commit intosymfony:3.3fromlinaori:fix/composer-resource-checker
Jun 16, 2017
Merged

Fixed composer resources between web/cli#23213

fabpot merged 1 commit intosymfony:3.3fromlinaori:fix/composer-resource-checker
Jun 16, 2017

Conversation

@linaori
Copy link
Contributor

QA
Branch?3.3
Bug fix?yes
New feature?no (reverts one)
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#23212
LicenseMIT
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.

@stof
Copy link
Member

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
Copy link
ContributorAuthor

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
Copy link
Member

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

Copy link
Member

@nicolas-grekasnicolas-grekas left a 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

@nicolas-grekasnicolas-grekas added this to the3.3 milestoneJun 16, 2017
@linaori
Copy link
ContributorAuthor

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
Copy link

@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
Copy link
Member

nicolas-grekas commentedJun 16, 2017
edited
Loading

I don't get why you say this is triggering a rebuild forevery request.
Only the first web request should do it, then the next ones should be "fresh" from this resource's pov, isn't it?

@linaori
Copy link
ContributorAuthor

linaori commentedJun 16, 2017
edited
Loading

@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.

sstok reacted with laugh emoji

@yannickl88
Copy link

yannickl88 commentedJun 16, 2017
edited
Loading

@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
Copy link
Member

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
Copy link
Member

Thank you@iltar.

@fabpotfabpot merged commit9e04712 intosymfony:3.3Jun 16, 2017
fabpot added a commit that referenced this pull requestJun 16, 2017
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
@fabpotfabpot mentioned this pull requestJul 4, 2017
@linaorilinaori deleted the fix/composer-resource-checker branchFebruary 8, 2019 13:38
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

6 participants

@linaori@stof@yannickl88@nicolas-grekas@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp