plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
246 stars 186 forks source link

Let CMFPlone depend on plone.app.caching #2376

Closed thet closed 1 year ago

thet commented 6 years ago

Currently plone.app.caching is only a dependency of the Plone distribution package (see: https://github.com/plone/Plone/blob/master/setup.py#L38 ). Not having plone.app.caching available doesn't even make use of ram caches and makes the site really slow.

I propose to add this dependency directly to CMFPlone.

/cc @plone/framework-team

mauritsvanrees commented 6 years ago

For a site that is only internally available, or where everyone is always authenticated, plone.app.caching may not give you much improvement, unless you spend a long time configuring it. I would always add it myself (same for plone.app.upgrade btw), but if we can keep it out of the dependencies, I think that would be nice.

thet commented 6 years ago

I saw response times of 20sec for each request vs 50ms for a site because apparently the Zope RAM cache wasn't leveraged without plone.app.caching installed. I think plone.app.caching makes sense also for internal sites unless something else was wrong. I also explicitly depend on plone.app.upgrade, e.g. for a separate service instance.

jensens commented 6 years ago

I agree with @thet. I always enabled caching even for Intranet sites. The RAM cache is one good reason, but also caching of CSS/JS in browser wont happen in those cases, w/o having sane rules caching delivered. W/o p.a.caching, on every request all JS/CSS and even fonts and icons are delivered.

plone.app.upgrade is a different case. I tend to have an separate service instance with ZMI access for those tasks.

hvelarde commented 6 years ago

probably is also time to review the default caching rules includes in plone.app.caching.

tomgross commented 6 years ago

I agree including p.a.caching is useful for almost 100% of real world usecases. Still I have mixed feelings of including it at as a hard dependency.

jensens commented 6 years ago

yes, the caching rules need a review. and yes, from a testing pov it is a good thing to have it separate. and most people use Plone as dependency i suppose.

but the killer argument against including it: more circular dependencies. https://github.com/plone/plone.app.caching/blob/master/plone/app/caching/browser/configure.zcml#L8 (those are not declared in setup.py)

jensens commented 1 year ago

architecture wise now it is an core-addon and thats good.