Skip to content

Conversation

@ro0NL
Copy link
Contributor

@ro0NL ro0NL commented Feb 23, 2022

Q A
Branch? 6.1
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

So we can export APP_RUNTIME_OPTIONS='{"disable_dotenv":true}' in prod

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 24, 2022

This is not a bugfix ;)
What about making SymfonyRuntime's constructor accept array|json-string instead?

@nicolas-grekas nicolas-grekas modified the milestones: 5.4, 6.1 Feb 24, 2022
@nicolas-grekas nicolas-grekas changed the base branch from 5.4 to 6.1 February 24, 2022 09:42
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

What about making SymfonyRuntime's constructor accept array|json-string instead?

Not a good idea as that would prevent merging with options generated by the plugin!

@ro0NL
Copy link
Contributor Author

ro0NL commented Feb 24, 2022

This is not a bugfix ;)

Are you saying APP_RUNTIME_OPTIONS is not a supported env var? but more like some internal global var?

IMHO it looks like an env var extension point ... thus should support string values actually. Thus a bugfix :)

Or how could i set an PHP array using env var?

We're not at 6.1 yet, so as a feature i will postpone this PR. And we patch the front controller manually for the time being (maybe forever), if that's the intended feature-usage currently.

@ro0NL
Copy link
Contributor Author

ro0NL commented Feb 24, 2022

So, tbe bugfix would be

$ APP_RUNTIME_OPTIONS={} bin/console 
PHP Warning:  A non-numeric value encountered in vendor/autoload_runtime.php on line 23
PHP Fatal error:  Uncaught Error: Unsupported operand types in vendor/autoload_runtime.php:23

@nicolas-grekas
Copy link
Member

more like some internal global var?

yes, that's been the intent so far, not reading from env.

@ro0NL
Copy link
Contributor Author

ro0NL commented Feb 24, 2022

then, what about renaming it to SF_RUNTIME_OPTIONS for 6.1?

@nicolas-grekas
Copy link
Member

I don't see why, better not to me 🤷

@nicolas-grekas
Copy link
Member

Can you add a test case?

@ro0NL
Copy link
Contributor Author

ro0NL commented Feb 24, 2022

I'll continue this with our php8/sf6 upgrade 👍

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants