-
Notifications
You must be signed in to change notification settings - Fork 71
Lazy-load commands #218
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
Lazy-load commands #218
Conversation
Great contribution! I'll have a look shortly but I think this will also fix #199 Thanks a lot for contributing. |
Thanks! The doc states that the |
any chance to change this? services:
_defaults:
public: true
AlgoliaSearch\Client:
arguments: ['%env(ALGOLIA_APP_ID)%', '%env(ALGOLIA_API_KEY)%'] the only way to use it like this is to define it at build time to, which requires a commit of the |
Unfortunately, that's unlikely to happen as the list command requires to access more than the command name (namespace, description, ...). Would be better to find a solution to lazy load the algolia client or another service up in the stack to prevent its instantiation when not strictly required. |
@ogizanagi thats exactly what I tried, but it is not working:
AlgoliaSearch\Client:
arguments: ['%env(ALGOLIA_APP_ID)%', '%env(ALGOLIA_API_KEY)%']
lazy: true do I need to activate the lazy stuff? |
No need to enable anything as soon as the ocramius proxy manager & bridge is installed. Check if any service is doing something with the client in the constructor or if a service is likely to use it during cache warmup. |
Damn, i guess this is breaking it, and I don't need to define the service on my own! WDYT of making the client lazy by default in this bundle? We could add This works for me: <?php
namespace AppBundle\DependencyInjection\Compiler;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
class AlgoliaCompilerPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
if ($container->has('algolia_client')) {
$definition = $container->getDefinition('algolia_client');
$definition->setLazy(true);
}
}
} |
Setting the lazy flag on the algolia client service definition and let the user install ocramius proxy-manager if he needs it looks a good workaround for now. |
yeah, you are right, we can set it directly on the service definition, no need for a compiler pass! I will provide a PR if you are fine with it @julienbourdeau |
@OskarStark @ogizanagi Thank you for looking into this! It would be best to move the conversation to #241 Sorry I didn't get back to you about this. |
so the algolia client isn't instantiated uselessly or too soon.
Some refs:
The
algolia_client
needs theALGOLIA_APP_ID
&ALGOLIA_API_KEY
env vars. But when building & releasing my application, this vars are not necessarily set (they are only set and available on the server, hence once the release is deployed). Each of these commands need the index or settings managers, which in turn require the client as dependency. Which means you'll get a:by running any command registered in the application.
By lazy-loading the algolia commands, none of their dependencies are instantiated unless really required.