silverstripe / silverstripe-versionfeed

Provides an RSS feed for global site changes
BSD 3-Clause "New" or "Revised" License
4 stars 13 forks source link

API Better implementation of caching / rate limiting #5

Closed tractorcow closed 10 years ago

tractorcow commented 10 years ago

Extension to allow rate limiting and configuration of caching behaviour.

Check the docs below for info. I've also added a 1.0.3 changelog with upgrading instructions.

The rate limiting and/or caching code is re-usable for purposes outside of versionfeed. :)

ref: CWPBUG-29

tractorcow commented 10 years ago

The above test cases won't pass in master until https://github.com/silverstripe/silverstripe-framework/pull/3047 is merged up into master as well.

tractorcow commented 10 years ago

Test cases are back and working. :)

tractorcow commented 10 years ago

Hold off on merging this for now; I'd like to extract the rate limit and caching mechanism into a separate module for cross-module use.

tractorcow commented 10 years ago

I've attempted to refactor out the necessary functionality into its own module, but I don't feel it was up to scratch. As far as versionedfeed is concerned, this pull request adequately solves the issue.

In other modules (such as fulltext search) we can write our own custom code to re-implement rate limiting in a way that makes sense to that module, rather than trying to over-generalise the solution.

Un-marking this as "do not merge" and once again am suggesting PR #5 as the best solution.

mateusz commented 10 years ago

Overall seems like an overkill solution, also without enough research into the source of the issue. Let's get it into a test deployment and load-test - doesn't make sense to spend much more time tweaking this stuff.

edit: clarified the intent, course of action recommended on internal CWPBUG jira issue.

tractorcow commented 10 years ago

Feedback implemented; Namespaced classes, changed default settings, updated docs.

tractorcow commented 10 years ago

Rate limiting now has a cooldown of 2 seconds.