Skip to content
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

clean up cache invalidation information on the cache chapter #4626

Merged
merged 4 commits into from
Jan 3, 2015
Merged

clean up cache invalidation information on the cache chapter #4626

merged 4 commits into from
Jan 3, 2015

Conversation

dbu
Copy link
Contributor

@dbu dbu commented Dec 11, 2014

Q A
Doc fix? yes
New docs? no
Applies to all
Fixed tickets -

The documentation on active cache invalidation is overly negative. I tried to make it more neutral and point to FOSHttpCacheBundle for further information.

$response = new Response();
if ($this->getStore()->purge($request->getUri())) {
$response->setStatusCode(200, 'Purged');
} else {
$response->setStatusCode(404, 'Not purged');
$response->setStatusCode(200, 'Not found');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a matter of taste, but i think asking for something to be deleted that is not there should not be considered an error.

@dbu
Copy link
Contributor Author

dbu commented Dec 11, 2014

btw, what do you think of moving the section on cache invalidation right after cache validation? we explain expiration and validation, then comes totally different stuff, and at the end we go back to invalidation as a 3rd option to manage cache validity.

@xabbuh
Copy link
Member

xabbuh commented Dec 11, 2014

btw, what do you think of moving the section on cache invalidation right after cache validation? we explain expiration and validation, then comes totally different stuff, and at the end we go back to invalidation as a 3rd option to manage cache validity.

Maybe move it after "Expiration and Validation"?

@dbu
Copy link
Contributor Author

dbu commented Dec 11, 2014

updated.

Maybe move it after "Expiration and Validation"?

exactly, i'd like to do that. the problem is that then the diff will not show what i changed in that section anymore. so i prefer to wait until the changes are considered ok, then i will move it up.

@xabbuh
Copy link
Member

xabbuh commented Dec 11, 2014

Looks good given the weak caching knowledge I have. 👍

@dbu
Copy link
Contributor Author

dbu commented Dec 11, 2014

thanks. i guess i wait until @weaverryan confirms its ok and then move the section up and let him merge it.

@@ -1063,9 +1069,15 @@ too far away in the future.
if you don't worry about invalidation, you can switch between reverse
proxies without changing anything in your application code.

Actually, all reverse proxies provide ways to purge cached data, but you
Copy link
Contributor

Choose a reason for hiding this comment

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

why removing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean the note that a cache can be invalidated? i can see to make that clear again for people really not familiar with caches...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rewrote the whole bit into a ..tip to say don't use invalidation if you can avoid it.

@weaverryan
Copy link
Member

@dbu 👍 from me - it reads really well, and I fully agree that we should be less negative and more pragmatic about cache invalidation.

So, feel free to move this up further now - I appreciate you making my life easier :). Also, It may make sense to mention FOSHttpCacheBundle near the top of the cache invalidation section. I would hate for someone to have a perfect use-case for it, but then read along and implement their own logic before realizing this tool is available to help them.

Thanks!

@dbu
Copy link
Contributor Author

dbu commented Dec 25, 2014

okay, moved the section to the end of the "validation and expiration" section and the link to fos httpcache up earlier into that section.

HTTP Expiration and Validation
------------------------------
HTTP Expiration, Validation and Invalidation
--------------------------------------------
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 to change this section title?

Copy link
Member

Choose a reason for hiding this comment

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

yes, as long as you make it BC. (so adding a .. _http-expiration-and-validation: anchor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, as long as you make it BC. (so adding a |..
_http-expiration-and-validation:| anchor

did that. looks a bit funny with the double anchor, however.

@@ -766,7 +776,7 @@ at some interval (the expiration) to verify that the content is still valid.
annotations. See the `FrameworkExtraBundle documentation`_.

.. index::
pair: Cache; Configuration
pair: Cache; Configuration
Copy link
Member

Choose a reason for hiding this comment

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

This should be reverted. We always use four spaces for indentation.

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 was looking at other files. http_fundamentals.rst for example is aligning on index:: as well, so they also have 3 spaces. as is the rest of this file.

@dbu
Copy link
Contributor Author

dbu commented Jan 1, 2015

good to merge? or any more feedback?

@xabbuh
Copy link
Member

xabbuh commented Jan 2, 2015

👍 Looks ready to be merged.

Happy new year @dbu!

@weaverryan
Copy link
Member

Thanks very much @dbu - this is clearly a big step forward. Cheers!

@weaverryan weaverryan merged commit 0accf63 into symfony:2.3 Jan 3, 2015
weaverryan added a commit that referenced this pull request Jan 3, 2015
…apter (dbu)

This PR was merged into the 2.3 branch.

Discussion
----------

clean up cache invalidation information on the cache chapter

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs? | no
| Applies to    | all
| Fixed tickets | -

The documentation on active cache invalidation is overly negative. I tried to make it more neutral and point to FOSHttpCacheBundle for further information.

Commits
-------

0accf63 cleanup cache book chapter
979034a move fos httpcache bundle tip up to beginning of invalidation section
2bddbb1 move invalidation up into expiration and validation section
d4d2236 clean up cache invalidation information on the cache chapter
@dbu dbu deleted the cleanup-cache-doc branch January 3, 2015 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants