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

[Runtime] Add environment variable APP_RUNTIME_MODE#51408

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

Conversation

@BaptisteContreras
Copy link
Contributor

QA
Branch?6.4
Bug fix?no
New feature?yes
Deprecations?no
TicketsFix#51340
LicenseMIT
Doc PRTODO

Based on#51340 (comment) I added the envvarAPP_RUNTIME_MODE in theautoload_runtime.php

Its purpose is to indicate in which mode the application is running (cli orweb).

I had a hard time figuring out how to test this. I finally decided to only test the priority to set the value ($_ENV > $_SERVER > default guessing).

During my test, I noticed that if I define a value forAPP_RUNTIME_MODE in$_ENV, and another value in$_SERVER, the output of$context['APP_RUNTIME_MODE'] in my application callable will be the value defined in$_SERVER, and the value$_ENV['APP_RUNTIME_MODE'] is untouched (which is normal). This is due to the way DotEnv loads and override envvars.

You can check this behavior insrc/Symfony/Component/Runtime/Tests/phpt/runtime_mode_from_env_with_server_set.php.

For me it seems OK because I can't see a way of having$_SERVER['APP_RUNTIME_MODE'] different from$_ENV['APP_RUNTIME_MODE']

I did not do the Doc PR, I first want to check if my implementation if OK for you

@ro0NL

This comment was marked as resolved.

@BaptisteContreras
Copy link
ContributorAuthor

APP_RUNTIME_TYPE could also be a good fit. I'd rather keepAPP_RUNTIME_MODE but I don't mind to change it if you, or others, want :)

Copy link
Member

@dunglasdunglas left a comment

Choose a reason for hiding this comment

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

This would be nice to have in 6.4.

BaptisteContreras reacted with thumbs up emoji
throw new TypeError(sprintf('Invalid return value: callable object expected, "%s" returned from "%s".', get_debug_type($app), $_SERVER['SCRIPT_FILENAME']));
}

$_ENV['APP_RUNTIME_MODE'] = $_ENV['APP_RUNTIME_MODE'] ?? $_SERVER['APP_RUNTIME_MODE'] ?? (in_array(\PHP_SAPI, ['cli', 'phpdbg'], true) ? 'cli' : 'web');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$_ENV['APP_RUNTIME_MODE']= $_ENV['APP_RUNTIME_MODE'] ?? $_SERVER['APP_RUNTIME_MODE'] ?? (in_array(\PHP_SAPI, ['cli', 'phpdbg'], true) ? 'cli' : 'web');
$_ENV['APP_RUNTIME_MODE']??= $_SERVER['APP_RUNTIME_MODE'] ?? (in_array(\PHP_SAPI, ['cli', 'phpdbg'], true) ? 'cli' : 'web');

BaptisteContreras reacted with thumbs up emoji
@dunglas
Copy link
Member

For#51661, we can add worker mode detection like that:

if (null === ($_ENV['APP_RUNTIME_MODE'] ??=$_ENV['APP_RUNTIME_MODE'] ??$_SERVER['APP_RUNTIME_MODE'] ??null)) {if ($_ENV['FRANKENPHP_WORKER'] ??$_SERVER['FRANKENPHP_WORKER']) {$_ENV['APP_RUNTIME_MODE'] ='worker';    }elseif (\PHP_SAPI ==='cli' || \PHP_SAPI ==='phpdbg') {$_ENV['APP_RUNTIME_MODE'] ='cli';    }else {$_ENV['APP_RUNTIME_MODE'] ='web';    }}

Support for other workers engine like RoadRunner will have to be done in the runtime themselves, because AFAIK they don't provide a way to detect if they are used at this point.

BaptisteContreras reacted with thumbs up emoji

@nicolas-grekasnicolas-grekas added the ❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze" labelOct 14, 2023
@BaptisteContreras
Copy link
ContributorAuthor

Hi@dunglas ! Thanks for the review. I updated the PR with your suggestions.

I slightly modified :

if ($_ENV['FRANKENPHP_WORKER'] ?? $_SERVER['FRANKENPHP_WORKER']) {

to

if ($_ENV['FRANKENPHP_WORKER'] ?? $_SERVER['FRANKENPHP_WORKER'] ?? false) {

For the case when$_SERVER['FRANKENPHP_WORKER'] does not exists.

dunglas reacted with thumbs up emoji

}

if (null === ($_ENV['APP_RUNTIME_MODE'] ??= $_SERVER['APP_RUNTIME_MODE'] ?? null)) {
if ($_ENV['FRANKENPHP_WORKER'] ?? $_SERVER['FRANKENPHP_WORKER'] ?? false) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you really need to have this specific check? Wouldn't that be something to put in FrankenPHP instead?

chalasr reacted with thumbs up emoji
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

We could do that also for CLI and SAPI. The main benefit of doing it here is to not have to duplicate this code in every runtime supporting FrankenPHP worker mode.

Copy link
Member

Choose a reason for hiding this comment

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

SettingAPP_RUNTIME_MODE in the bridge would be a better demonstration of how this have to be configured. Otherwise we'll receive PR to add Bref or React support in this file.

@nicolas-grekas
Copy link
Member

While reviewing this PR, I thought another approach might be preferable, see#52079.

@BaptisteContreras
Copy link
ContributorAuthor

While reviewing this PR, I thought another approach might be preferable, see#52079.

Your implementation seems indeed more flexible !

@nicolas-grekas
Copy link
Member

Thanks for pushing this forward@BaptisteContreras!

fabpot added a commit that referenced this pull requestOct 20, 2023
…`kernel.runtime_mode.*`, all set from env var `APP_RUNTIME_MODE` (nicolas-grekas)This PR was merged into the 6.4 branch.Discussion----------[HttpKernel] Add parameters `kernel.runtime_mode` and `kernel.runtime_mode.*`, all set from env var `APP_RUNTIME_MODE`| Q             | A| ------------- | ---| Branch?       | 6.4| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       |Fix#51340| License       | MIT| Doc PR        | TODOAlternative 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 (eghttps://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.Commits-------7c70aec [HttpKernel] Add parameters `kernel.runtime_mode` and `kernel.runtime_mode.*`, all set from env var `APP_RUNTIME_MODE`
@rustatian
Copy link

For#51661, we can add worker mode detection like that:

if (null === ($_ENV['APP_RUNTIME_MODE'] ??= $_ENV['APP_RUNTIME_MODE'] ?? $_SERVER['APP_RUNTIME_MODE'] ?? null)) {    if ($_ENV['FRANKENPHP_WORKER'] ?? $_SERVER['FRANKENPHP_WORKER']) {        $_ENV['APP_RUNTIME_MODE'] = 'worker';    } elseif (\PHP_SAPI === 'cli' || \PHP_SAPI === 'phpdbg') {        $_ENV['APP_RUNTIME_MODE'] = 'cli';    } else {        $_ENV['APP_RUNTIME_MODE'] = 'web';    }}

Support for other workers engine like RoadRunner will have to be done in the runtime themselves, because AFAIK they don't provide a way to detect if they are used at this point.

Hey 👋🏻
RR provides a way to detect (inside a worker) if it is used, per-plugin. For instance, if HTTP plugin is used->RR_MODE env variable would be set toHTTP:https://roadrunner.dev/docs/php-environment/current#default-env-values-in-php-workers

@dunglas
Copy link
Member

Hi@rustatian, thanks for the hint!

We decided to move this part in the dedicated adapters provided by php-runtime. If you mind son't hesitate to open a PR for RoadRunner!https://github.com/php-runtime/runtime

@rustatian
Copy link

With pleasure,@dunglas 😄
I'll ask our PHP team to do that, since I'm not a PHP dev 😢

dunglas and GromNaN reacted with heart emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@GromNaNGromNaNGromNaN left review comments

@chalasrchalasrchalasr left review comments

@dunglasdunglasdunglas approved these changes

Assignees

No one assigned

Labels

FeatureRuntime❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze"Status: Reviewed

Projects

None yet

Milestone

6.4

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.

8 participants

@BaptisteContreras@ro0NL@dunglas@nicolas-grekas@rustatian@GromNaN@chalasr@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp