Skip to content

Conversation

@iamluc
Copy link
Contributor

@iamluc iamluc commented Apr 22, 2014

This PR is another try to use non-local file resources in ConfigCache, like #7230

There are 2 main changes:

  • A new interface "MutableResourceInterface" which extends "ResourceInterface" for compatibility and adds a new method "isMutable()".
    The metafile is now created if at least one resource is mutable, and not according to the debug flag like before.
  • "isFresh()" method can take as first argument an instance of "RefresherInterface" which has only one method: "refresh(MutableResourceInterface $resource)"
    It serves for example to reinject the database connexion after unserializing ressources

Compatibility should be maintained.

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #7230
License MIT
Doc PR

TODO:

  • Comments

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not make sense. If you don't lazy-load the refresher, you should inject it directly instead of injecting a service id

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, as we already have the container, it seemed strange to inject one more object....
But I will change this.

@iamluc iamluc added the Config label Apr 28, 2014
@fabpot
Copy link
Member

fabpot commented Sep 24, 2014

After talking with @iamluc about this PR, I'm going to close it as this is a use case I don't want to support. To make a long story short, the ConfigCache class should only be used for resources that do need to be checked for changes after they are compiled. So, the concept of a mutable resource is out of this scope.

@fabpot fabpot closed this Sep 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants