8000 [DI] Allow reseting of env vars loaded by EnvVarLoaderInterface implementations · Issue #54627 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] Allow reseting of env vars loaded by EnvVarLoaderInterface implementations #54627

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
faizanakram99 opened this issue Apr 16, 2024 · 4 comments · Fixed by #54666
Closed

[DI] Allow reseting of env vars loaded by EnvVarLoaderInterface implementations #54627

faizanakram99 opened this issue Apr 16, 2024 · 4 comments · Fixed by #54666
Labels
DependencyInjection Feature

Comments

@faizanakram99
Copy link
Contributor
faizanakram99 commented Apr 16, 2024 •
edited
Loading

Description

Services tagged with container.env_var_loader are loaded by EnvVarProcessor to load env vars dynamically. Right now there is no way to reset such env vars once they're loaded mainly because they're cached in memory once loaded . This seems by design to improve performance, however, it limits the usage of EnvVarLoader in messenger context say with a multi-tenant app where a single worker can handle messages from various tenants.

This slack thread might give a bit more context https://symfony-devs.slack.com/archives/C8WHX21K7/p1713264333709759

Possible solution:
EnvVarProcessor can possibly implement reset interface and allow reseting some env vars (maybe via a config option?), especially the ones which are loaded by EnvVarLoaderInterface implementations, since the env vars loaded by EnvVarLoaderInterface aren't actual env vars and are dynamic (or runtime) by definition to begin with, it makes sense to allow resetting them.

This would allow using symfony messenger easily in a multi-tenant app where env vars are loaded based on tenant id.

Example

No response
@faizanakram99 faizanakram99 changed the title [DependencyInjection] Allow reseting of env vars loaded by EnvVarLoaderInterface implementations [DI] Allow reseting of env vars loaded by EnvVarLoaderInterface implementations Apr 16, 2024
@xabbuh xabbuh added the Feature label Apr 17, 2024
@nicolas-grekas
Copy link
Member

Looks like a good idea to me. Up for a PR?

@faizanakram99
Copy link
Contributor Author

Yes, I can try. Thanks.

@tucksaun
Copy link
Contributor

Would that work in the end?
It seems to me the env vars are cached higher in the stack (in the container) to cache across different processors (see

symfony/src/Symfony/Component/DependencyInjection/Container.php

Lines 339 to 372 in 5ae5791

protected function getEnv(string $name): mixed
{
if (isset($this->resolving[$envName = "env($name)"])) {
throw new ParameterCircularReferenceException(array_keys($this->resolving));
}
if (isset($this->envCache[$name]) || \array_key_exists($name, $this->envCache)) {
return $this->envCache[$name];
}
if (!$this->has($id = 'container.env_var_processors_locator')) {
$this->set($id, new ServiceLocator([]));
}
$this->getEnv ??= $this->getEnv(...);
$processors = $this->get($id);
if (false !== $i = strpos($name, ':')) {
$prefix = substr($name, 0, $i);
$localName = substr($name, 1 + $i);
} else {
$prefix = 'string';
$localName = $name;
}
$processor = $processors->has($prefix) ? $processors->get($prefix) : new EnvVarProcessor($this);
if (false === $i) {
$prefix = '';
}
$this->resolving[$envName] = true;
try {
return $this->envCache[$name] = $processor->getEnv($prefix, $localName, $this->getEnv);
} finally {
unset($this->resolving[$envName]);
}
}
)

@faizanakram99
Copy link
Contributor Author

Would that work in the end?
It seems to me the env vars are cached higher in the stack (in the container) to cache across different processors (see

symfony/src/Symfony/Component/DependencyInjection/Container.php

Lines 339 to 372 in 5ae5791

protected function getEnv(string $name): mixed
{
if (isset($this->resolving[$envName = "env($name)"])) {
throw new ParameterCircularReferenceException(array_keys($this->resolving));
}
if (isset($this->envCache[$name]) || \array_key_exists($name, $this->envCache)) {
return $this->envCache[$name];
}
if (!$this->has($id = 'container.env_var_processors_locator')) {
$this->set($id, new ServiceLocator([]));
}
$this->getEnv ??= $this->getEnv(...);
$processors = $this->get($id);
if (false !== $i = strpos($name, ':')) {
$prefix = substr($name, 0, $i);
$localName = substr($name, 1 + $i);
} else {
$prefix = 'string';
$localName = $name;
}
$processor = $processors->has($prefix) ? $processors->get($prefix) : new EnvVarProcessor($this);
if (false === $i) {
$prefix = '';
}
$this->resolving[$envName] = true;
try {
return $this->envCache[$name] = $processor->getEnv($prefix, $localName, $this->getEnv);
} finally {
unset($this->resolving[$envName]);
}
}
)

Indeed, those would need to be reset as well, thanks I had missed that.

@faizanakram99 faizanakram99 mentioned this issue Apr 18, 2024
Merged
@fabpot fabpot closed this as completed in e6c875e May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DependencyInjection Feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[DependencyInjection] Reset env vars when resetting the container
5 participants
@nicolas-grekas @tucksaun @xabbuh @faizanakram99 @carsonbot
0