8000 [HttpKernel] Add parameters `kernel.runtime_mode` and `kernel.runtime_mode.*`, all set from env var `APP_RUNTIME_MODE` by nicolas-grekas · Pull Request #52079 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Conversation

nicolas-grekas
Copy link
Member
@nicolas-grekas nicolas-grekas commented Oct 16, 2023
Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #51340
License MIT
Doc PR TODO

Alternative to #51408. I think this approach is simpler and more powerful.

Here, we ensure that the kernel always provides a new kernel.runtime_mode parameter. This parameter is an array derived by default from the APP_RUNTIME_MODE env var, using the query_string processor.

This also creates 3 new parameters that should be the most common: kernel.runtime_mode.web, kernel.runtime_mode.cli, and kernel.runtime_mode.worker.

A long-running server would typically set APP_RUNTIME_MODE to web=1&worker=1 or web=0&worker=1 when appropriate (eg https://github.com/php-runtime/frankenphp-symfony/ should do so when FRANKENPHP_WORKER is set.)

I screened the codebase and updated them all except cache pools (where the SAPI is used to enable/disable locking) and error renderers (where the SAPI is used to turn html-rendering on/off.) These require more work that could be done later on. There are a few other remaining usages of PHP_SAPI but these look not appropriate for the new flag.

@carsonbot carsonbot added this to the 6.4 milestone Oct 16, 2023
@nicolas-grekas nicolas-grekas added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Oct 16, 2023
@nicolas-grekas nicolas-grekas force-pushed the hk-runtime_mode branch 2 times, most recently from 5e777ad to 1387951 Compare October 16, 2023 20:18
@stof
Copy link
Member
stof commented Oct 17, 2023

shouldn't the error rendering depend on web_mode ? For a long-running server, we would want to provide the web error rendering.

@nicolas-grekas
Copy link
Member Author

shouldn't the error rendering depend on web_mode

yes, but it's too much work for me for now, could be handled later on (same for cache locking, see PR description)

@nicolas-grekas nicolas-grekas changed the title [HttpKernel] Add parameter kernel.runtime_mode, defined as %env(default:container.runtime_mode:APP_RUNTIME_MODE)% [HttpKernel] Add parameters kernel.runtime_mode and kernel.runtime_mode.*, all set from env var APP_RUNTIME_MODE Oct 18, 2023
@nicolas-grekas
Copy link
Member Author

PR updated. No special service anymore, only parameters. Here is the new description:

Here, we ensure that the kernel always provides a new kernel.runtime_mode parameter. This parameter is an array derived by default from the APP_RUNTIME_MODE env var, using the query_string processor.

This also creates 3 new parameters that should be the most common: kernel.runtime_mode.web, kernel.runtime_mode.cli, and kernel.runtime_mode.worker.

A long-running server would typically set APP_RUNTIME_MODE to web=1&worker=1 or web=0&worker=1 when appropriate (eg https://github.com/php-runtime/frankenphp-symfony/ should do so when FRANKENPHP_WORKER is set.)

}

if (\in_array(\PHP_SAPI, ['cli', 'phpdbg'], true) && !filter_var(\ini_get('apc.enable_cli'), \FILTER_VALIDATE_BOOL)) {
if ('cli' === \PHP_SAPI && !filter_var(\ini_get('apc.enable_cli'), \FILTER_VALIDATE_BOOL)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aligning with other checks we have in place for APCu (not need to enable it for phpdbg/embed)

$export = explode('0 => ', substr(rtrim($export, " ]\n"), 2, -1), 2);

if ($hasEnum || preg_match("/\\\$container->(?:getEnv\('(?:[-.\w\\\\]*+:)*+\w++'\)|targetDir\.'')/", $export[1])) {
if ($hasEnum || preg_match("/\\\$container->(?:getEnv\('(?:[-.\w\\\\]*+:)*+\w*+'\)|targetDir\.'')/", $export[1])) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change allows to derivate dynamic parameters from other ones, eg %env(key:foo:default:bar:)% extracts key foo from parameter bar (which is supposed to be an array here)

…_mode.*`, all set from env var `APP_RUNTIME_MODE`
} elseif (\function_exists('litespeed_finish_request') && !$this->debug) {
litespeed_finish_request();
} else {
Response::closeOutputBuffers(0, true);
Copy link
Member Author
@nicolas-grekas nicolas-grekas Oct 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike the logic in $response->send(), we call this unconditionally. The reason is that here, in the runtime, we know we want to flush whatever the SAPI.

@fabpot
Copy link
Member
fabpot commented Oct 20, 2023

Thank you @nicolas-grekas.

@fabpot fabpot merged commit b6418aa into symfony:6.4 Oct 20, 2023
@nicolas-grekas nicolas-grekas deleted the hk-runtime_mode branch October 20, 2023 17:08
This was referenced Oct 21, 2023
alexander-schranz pushed a commit to php-runtime/runtime that referenced this pull request Oct 30, 2023
Nyholm pushed a commit to php-runtime/frankenphp-symfony that referenced this pull request Oct 30, 2023
nicolas-grekas added a commit that referenced this pull request Oct 12, 2025
…t (HypeMC)

This PR was merged into the 6.4 branch.

Discussion
----------

[DebugBundle] Wire `DumpDataCollector`'s `webMode` argument

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | maybe
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

I've noticed that the `DumpDataCollector::$webMode` that was added in #52079 was never wired. I'm not sure if this was intentional or an oversight.

Commits
-------

7644e9d [DebugBundle] Wire `DumpDataCollector`'s `webMode` argument
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature HttpKernel ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" Status: Reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Function to check if the kernel is loaded via the console or a web request.

5 participants

0