Skip to content

[DependencyInjection] allow decorating a lazy service using a final class if it's wired by an interface #53436

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
dkarlovi opened this issue Jan 5, 2024 · 5 comments
Closed

[DependencyInjection] allow decorating a lazy service using a final class if it's wired by an interface #53436

dkarlovi opened this issue Jan 5, 2024 · 5 comments
Labels
DependencyInjection

Comments

@dkarlovi
Copy link
Contributor

dkarlovi commented Jan 5, 2024

Description

I'm implementing https://github.com/Unleash/unleash-client-symfony which has an interface used as the client and a default implementation which is final, all of which is good and correct.

The bundle also comes with Twig integration so you can use feature flag directly in Twig, again good stuff.

I'm using k8s and obviously want to manage the config via secrets, like so

unleash_symfony_client:
    app_url: '%env(string:key:app_url:json:file:UNLEASH_DSN_FILE)%'
    instance_id: '%env(string:key:instance_id:json:file:UNLEASH_DSN_FILE)%'
    app_name: '%env(APP_NAME)%'

Unfortunately, when pre-building the app with cache:warmup, this means the service is attempted to be created which fails (known issue):

1.010 
1.010 In EnvVarProcessor.php line 205:
1.010                                                                           
1.010   [Symfony\Component\DependencyInjection\Exception\EnvNotFoundException]  
1.010   Environment variable not found: "UNLEASH_DSN_FILE".                     
1.010                                                                           
1.010 
1.010 Exception trace:
1.010   at /app/vendor/symfony/dependency-injection/EnvVarProcessor.php:205
1.011  Symfony\Component\DependencyInjection\EnvVarProcessor->getEnv() at /app/vendor/symfony/dependency-injection/Container.php:382
1.011  Symfony\Component\DependencyInjection\Container->getEnv() at /app/vendor/symfony/dependency-injection/EnvVarProcessor.php:141
1.011  Symfony\Component\DependencyInjection\EnvVarProcessor->getEnv() at /app/vendor/symfony/dependency-injection/Container.php:382
1.011  Symfony\Component\DependencyInjection\Container->getEnv() at /app/vendor/symfony/dependency-injection/EnvVarProcessor.php:165
1.011  Symfony\Component\DependencyInjection\EnvVarProcessor->getEnv() at /app/vendor/symfony/dependency-injection/Container.php:382
1.011  Symfony\Component\DependencyInjection\Container->getEnv() at /app/vendor/symfony/dependency-injection/EnvVarProcessor.php:74
1.011  Symfony\Component\DependencyInjection\EnvVarProcessor->getEnv() at /app/vendor/symfony/dependency-injection/Container.php:382
1.011  Symfony\Component\DependencyInjection\Container->getEnv() at /app/vendor/symfony/dependency-injection/EnvVarProcessor.php:165
1.011  Symfony\Component\DependencyInjection\EnvVarProcessor->getEnv() at /app/vendor/symfony/dependency-injection/Container.php:382
1.011  Symfony\Component\DependencyInjection\Container->getEnv() at /app/var/cache/prod/ContainerAOpqf4e/App_Infrastructure_Symfony_KernelProdContainer.php:1174
1.011  ContainerAOpqf4e\App_Infrastructure_Symfony_KernelProdContainer::getUnleash_Client_UnleashService() at /app/var/cache/prod/ContainerAOpqf4e/getTwigService.php:100
1.011  ContainerAOpqf4e\getTwigService::do() at /app/var/cache/prod/ContainerAOpqf4e/App_Infrastructure_Symfony_KernelProdContainer.php:114
1.011  ContainerAOpqf4e\App_Infrastructure_Symfony_KernelProdContainer->load() at /app/vendor/symfony/dependency-injection/Container.php:403
1.011  Symfony\Component\DependencyInjection\Container->getService() at /app/vendor/symfony/dependency-injection/Argument/ServiceLocator.php:40
1.011  Symfony\Component\DependencyInjection\Argument\ServiceLocator->get() at /app/vendor/symfony/twig-bundle/CacheWarmer/TemplateCacheWarmer.php:43
1.011  Symfony\Bundle\TwigBundle\CacheWarmer\TemplateCacheWarmer->warmUp() at /app/vendor/symfony/http-kernel/CacheWarmer/CacheWarmerAggregate.php:108
1.011  Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerAggregate->warmUp() at /app/vendor/symfony/framework-bundle/Command/CacheWarmupCommand.php:75
1.011  Symfony\Bundle\FrameworkBundle\Command\CacheWarmupCommand->execute() at /app/vendor/symfony/console/Command/Command.php:326
1.011  Symfony\Component\Console\Command\Command->run() at /app/vendor/symfony/console/Application.php:1096
1.011  Symfony\Component\Console\Application->doRunCommand() at /app/vendor/symfony/framework-bundle/Console/Application.php:126
1.011  Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() at /app/vendor/symfony/console/Application.php:324
1.011  Symfony\Component\Console\Application->doRun() at /app/vendor/symfony/framework-bundle/Console/Application.php:80
1.011  Symfony\Bundle\FrameworkBundle\Console\Application->doRun() at /app/vendor/symfony/console/Application.php:175
1.011  Symfony\Component\Console\Application->run() at /app/vendor/symfony/runtime/Runner/Symfony/ConsoleApplicationRunner.php:49
1.011  Symfony\Component\Runtime\Runner\Symfony\ConsoleApplicationRunner->run() at /app/vendor/autoload_runtime.php:29
1.011  require_once() at /app/bin/console:15
1.011 
1.011 cache:warmup [--no-optional-warmers]

Reason is:

  1. I'm warming up Symfony "cache" (but actually building Symfony)
  2. Twig templates need to be built
  3. Twig environment requested
  4. Twig Unleash extension requested
  5. Unleash client requested
  6. cannot be configured at build time, it's runtime

Typically, the solution is to make the client lazy since it will not actually be used, it's only needed to satisfy the dep enough to build the templates.

But here, since the client is final, I get

Cannot generate lazy proxy: class "Unleash\Client\Bundle\Unleash\UnleashDecorator" is final.

But, the Twig extension is actually not requesting that class, it's requesting by interface Unleash\Client\Unleash.

Idea

The solution is to decorate the service with a non-final class. But, Symfony could do that too.

The idea here is to

  1. confirm the wiring is always done by interface
  2. if so, allow making the service lazy by auto-decorating it with proxy implementing the interface, not extending the class

Example

No response
@nicolas-grekas
Copy link
Member

Twig Unleash extension requested

Shouldn't you split the extension from its runtime and all the troubles are fixed?

@dkarlovi
Copy link
Contributor Author

dkarlovi commented Feb 1, 2024

@nicolas-grekas this is 3rd party code, not sure what steps you're suggesting here? 🤔

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Mar 7, 2024 •
edited
Loading

Submitting a PR to them? :)
Alternatively, aren't you looking for https://symfony.com/doc/current/service_container/lazy_services.html#interface-proxifying ?

@dkarlovi
Copy link
Contributor Author

dkarlovi commented Mar 8, 2024

Submitting a PR to them? :)

But what would the even PR do? 🤔 They did everything correctly AFAIK, it's Symfony lacking the "build step" which would cleanly separate building Twig templates (no env var access allowed) vs using Twig templates (env var access allowed), which we were discussing in #40794.

Alternatively, aren't you looking for https://symfony.com/doc/current/service_container/lazy_services.html#interface-proxifying ?

TIL, yes I am, thanks! 👍 I guess what I had in mind is already possible, maybe the PR to them could do that until Symfony fixes the build time vs runtime conundrum.

@nicolas-grekas
Copy link
Member

buildtime vs runtime is what twig extension runtimes where created for, that lib should leverage them.

@dkarlovi dkarlovi mentioned this issue May 29, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DependencyInjection
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dkarlovi @nicolas-grekas @carsonbot