10000 [RFC][WIP][2.3] Cache Bundle by dlsniper · Pull Request #5903 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[RFC][WIP][2.3] Cache Bundle #5903

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
wants to merge 1 commit into from
Closed

[RFC][WIP][2.3] Cache Bundle #5903

wants to merge 1 commit into from

Conversation

dlsniper
Copy link
Contributor
@dlsniper dlsniper commented Nov 3, 2012

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1513, #3211
License of the code: MIT
Documentation PR: ~
Todo:

  • feedback from GH;
  • add create documentation PR for the component;
  • fix any CS break;
  • fix Configuration WTFs as it's the first time I use the Configuration component
  • optimizations?

Hello,

This is the bundle that is 'missing' from the previous PR and it is also with the Request For Comments status.

I'll try and add the remaining things from the TODO list in the specified order.

At least for now I don't care about coding style breaks and so on, I think it's the least important thing right now (don't worry, I'll fix everything, if anything, before merging), so please don't comment about it!

As a last thing, I really want to finish this in time for 2.2 release/feature freeze so if you want to help me out, any and all PRs are welcomed.

Thank you!

Configuration:

cache:
    drivers:
        apc:          ~
        memcached:
            server:
                host: 127.0.0.1
                port: 11211
    instances:
        apc1:
            type:     apc
        memcached1:
            type:     memcached
        memcached2:
            type:     memcached
            server:
                host: 127.0.0.1
                port: 11212
$cacheObject = new CacheItem('demo3', 'demo2.4', 600);

$this->get('cache.apc1')->set($cacheObject);
var_dump($this->get('cache.apc1')->get('demo3'));

$this->get('cache.memcached1')->set($cacheObject);
var_dump($this->get('cache.memcached1')->get('demo3'));

Demo available here: http://sf2demo.rodb.ro/app_dev.php/

@dlsniper dlsniper mentioned this pull request Nov 3, 2012
Closed
stof
stof reviewed Nov 3, 2012
View reviewed changes
src/Symfony/Bundle/CacheBundle/Factory/CacheFactory.php
$cacheDrivers = $config['cache.drivers'];
$cacheInstances = $config['cache.instances'];

$this->manager = new $managerClass();
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. You should use the container to configure the manager and pass a CacheManager to the constructor.

@dlsniper
Copy link
Contributor Author
dlsniper commented Nov 6, 2012

Since all the job is now done by Cache\Cache.php I'm not sure if we need to have the CacheBundle\Factory\CacheFactory.php file as it's just plain useless atm

stof
stof reviewed Nov 6, 2012
View reviewed changes
src/Symfony/Bundle/CacheBundle/Resources/views/Collector/cache.html.twig

{% block panel %}
<h2>Cache panel information</h2>
{% endblock %} No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Implementing a fake template for the data collector before implementing any cache profiling seems really weird to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to know what I'll need in profiler before making it. If my time will allow it, I'll do it tomorrow or during the next weekend.

@stof
Copy link
Member
stof commented Nov 6, 2012

@dlsniper indeed, there is no reason to have this CacheFactory

@asm89
Copy link
Contributor
asm89 commented Nov 7, 2012

There is also a CacheBundle implementation pending here: #3225.

@dlsniper
Copy link
Contributor Author
dlsniper commented Nov 7, 2012

@asm89 thanks, I know about it but @vicb can't take care of it right now and I'm using a simpler approach right now that his.

@dlsniper
Copy link
Contributor Author

This is a bit of a mess right now, especially on the config part but I wanted to get it work again. More changes soon.

@fabpot
Copy link
Member
fabpot commented Mar 23, 2013

Closing as there are 4 opened PR on this topic. I would prefer that we first discuss the interface and what we want before starting coding. I suggest 8000 that this discussion happens on the Symfony dev mailing-list. Also, we need to discuss what to do with the current discussion on the FIG group mailing-list.

@fabpot fabpot closed this Mar 23, 2013
@fabpot
Copy link
Member
fabpot commented Mar 23, 2013

https://groups.google.com/forum/#!topic/symfony-devs/EJr6XhawJTE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dlsniper @stof @asm89 @fabpot
0