Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

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

Comments

@faizanakram99
Copy link
Contributor

faizanakram99 commented Apr 16, 2024

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

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

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 added a commit to faizanakram99/symfony that referenced this issue Apr 18, 2024
faizanakram99 added a commit to faizanakram99/symfony that referenced this issue Apr 18, 2024
faizanakram99 added a commit to faizanakram99/symfony that referenced this issue Apr 18, 2024
faizanakram99 added a commit to faizanakram99/symfony that referenced this issue Apr 18, 2024
faizanakram99 added a commit to faizanakram99/symfony that referenced this issue Apr 22, 2024
faizanakram99 added a commit to faizanakram99/symfony that referenced this issue Apr 22, 2024
faizanakram99 added a commit to faizanakram99/symfony that referenced this issue Apr 23, 2024
faizanakram99 added a commit to faizanakram99/symfony that referenced this issue Apr 29, 2024
@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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants