Skip to content

Have a common way to override services per storage #11460

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

Closed
dawehner opened this issue Jul 24, 2014 · 15 comments
Closed

Have a common way to override services per storage #11460

dawehner opened this issue Jul 24, 2014 · 15 comments

Comments

@dawehner
Copy link
Contributor

In Drupal we want to deal with the problem that some services are bound to the database, for example a lock service. Per default this is using the database but you might want to use something else.

You can for sure always write a compiler pass and replace the service with one for your wanted backend like redis (just as example). This is quite annoying if you as a developer have a lot of services, or you just want to switch the storage as site administrator.

We came up with a schema which basically allows you to set a tag on these services (https://www.drupal.org/node/2302617). A pass will then try to look up "$backend.$service" services, with $backend being a configurable container parameter. This for example allows us to write mysql specific optimizations into "mysql.$service".

I wonder whether you are interested in a generic functionality. Given the structure I guess this should be part of some bundle, so it is not reusable but at least we would have a common functionality.

@merk
Copy link
Contributor

merk commented Jul 24, 2014

Bundles already handle this by defining a set of backend specific services as private, and setting an alias for the appropriate one to a primary service id based on configuration. Private services not referenced get removed by a cleanup compiler pass.

https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/Resources/config/orm.xml#L8 defines a default service, and the extension class then sets an alias based on its configuration: https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/DependencyInjection/FOSUserExtension.php#L48

This lets the developer define their own service and tell the extension about it and that it should become the manager.

@dawehner
Copy link
Contributor Author

What the code of drupal allows is something like the following:

lock:
class: Drupal\Core\Lock\Lock...
myqsl.lock:
class: Drupal\Core\Lock\Lock...
sqlite.lock:
class: Drupal\Core\Lock\Lock...

As a site adminstrator you just need to set 'default_backend' and mysql.lock will be automatically aliased to lock

@merk
Copy link
Contributor

merk commented Jul 24, 2014

The code I linked achieves the same thing.

@dawehner
Copy link
Contributor Author

I don't see how this is done in a generic way for all possible storages.
Can you enlighten me?

On Thu, Jul 24, 2014 at 11:40 AM, Tim Nagel notifications@github.com
wrote:

The code I linked achieves the same thing.


Reply to this email directly or view it on GitHub
#11460 (comment).

@kingcrunch
Copy link
Contributor

@dawehner You define every possible storage as private service and you use a (non-yet used) service-id as dependency for every occurence, where one of this storages is required. In your Extension-class you read the configuration and set this not yet known service-ID as an alias onto one of the existing (private) services depending on the configuration.

$storageName = $config['storage']; // For example "redis"
$container->setAlias('foo_bar.storage', "foo_bar.storage.$storageName");

or if you want it super-generic use the full qualified service id in the configuration instead (that's what you can see in the links given by @merk )

@dawehner
Copy link
Contributor Author

So it is not automatic as proposed in the issue ... This is not a support
question
On Jul 24, 2014 11:47 AM, "Sebastian Krebs" notifications@github.com
wrote:

@dawehner https://github.com/dawehner You define every possible storage
as private server and you use a (non-yet used) service-id for every
occurence, where one of this storage should appear. In your Extension-class
you read the configuration and set this not yet known service-ID as an
alias onto one of the existing (private) services depending on the
configuration.

$storageName = $config['storage']; // For example "redis"$container->setAlias('foo_bar.storage', "foo_bar.storage.$storageName");


Reply to this email directly or view it on GitHub
#11460 (comment).

@merk
Copy link
Contributor

merk commented Jul 24, 2014

It can be handled by a simple compiler pass that looks for a parameter and handles this without tags. I'm not talking about support - I'm mentioning that this is already possible with very little additional effort on top of a container.

@dawehner
Copy link
Contributor Author

I totally don't argue against that, I think we have kind of a communication problem.

I just wanted to ask whether symfony would like to have such an automatic backend switching system included, maybe in FrameworkBundle.

@stof
Copy link
Member

stof commented Aug 14, 2014

I don't think it can be in FrameworkBundle. You can have different bundles supporting to choose their backend while the choice in one is totally independant from the choice in others (potentially with different values for the choice items btw). Making something generic in FrameworkBundle would probably end up being more complex than having the bundle handling the config it needs.

@fabpot
Copy link
Member

fabpot commented Oct 10, 2014

Just talked with @Crell about this one.

So, basically, that's interesting but if it should be more generic than the Drupal use case.

The problem Drupal is trying to solve is to automatically create aliases based on some parameter value. We could imagine a tag that makes this easier to achieve, based on some conventions/options:

lock:
    class: Whatever
    tags:
        - { name: auto_alias, { parameter_name: default_backend, format: "%s.lock" }  }

parameters:
    default_backend: mysql

And the compiler would make lock an alias for mysql.lock.

@Crell
Copy link
Contributor

Crell commented Nov 19, 2014

I like the more generic format @fabpot suggests. It's a little bit more wordy for the person setting up the service but gives more flexibility. (One could have multiple "classes" of swappable services this way, which is nice.) I don't know if we could switch Drupal's syntax to that now that we're in beta but I would not object to doing so.

@javiereguiluz
Copy link
Member

It seems that we all agree on the last @fabpot proposal. So the question is, who's gonna prepare the PR for this change? @Crell do you prefer if some Drupal developers take this responsibility? @fabpot do you prefer to do this PR yourself?

@fabpot
Copy link
Member

fabpot commented Nov 19, 2014

@javiereguiluz I don't have any special recommendations here, except that it's going to be in 2.7.

@dawehner
Copy link
Contributor Author

Which hopefully will be the version in which d8 ships

@catch56
Copy link

catch56 commented Nov 20, 2014

I'm OK with an API change in Drupal to use the Symfony format if that's added since the one we added isn't used much yet. However the change would need to be made before we start supporting an upgrade path between betas (which doesn't have an explicit timeframe - it's tied to issues fixed).

fabpot added a commit that referenced this issue Feb 12, 2015
This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes #12526).

Discussion
----------

Add an auto_alias compiler pass

Discussion see #11460

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | [#11460]
| License       | MIT
| Doc PR        |

Commits
-------

d7432c0 Add an auto_alias compiler pass
@fabpot fabpot closed this as completed Feb 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants