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

Add a list of default matched status code #229

Merged
merged 2 commits into from Jul 5, 2015
Merged

Add a list of default matched status code #229

merged 2 commits into from Jul 5, 2015

Conversation

tgalopin
Copy link
Contributor

@tgalopin tgalopin commented Jul 3, 2015

I think it's not very clear for the moment what status code are matched by a rule. I propose to add the list in the documentation, to avoid other problems like #224 (comment).

@@ -128,8 +128,11 @@ regular expressions.

**type**: ``array``

A list of additional HTTP status codes of the response for which to also apply
the rule.
By defaut, a rule will only match cacheable status codes: 200, 203, 300, 301,
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please link "cacheable status codes" to http://tools.ietf.org/html/rfc7231#page-48 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, and travis noted that you have a spelling error: s/defaut/default/

Copy link
Member

Choose a reason for hiding this comment

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

In that RFC, the list is longer: ‘e.g., 200, 203, 204, 206, 300, 301, 404, 405, 410, 414, and 501 in this specification’. Should we adhere to the RFC and cache those status code by default, too? So including 204 that caused confusion in #224.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, indeed rfc2616 was more restrictive: http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.4

lets increase the default list but also fix the behaviour that invalidation is possible on any status < 400.

@ddeboer
Copy link
Member

ddeboer commented Jul 5, 2015

Merging this, as it correctly reflects the current situation.

Let’s discuss the configurability and up-to-date status codes (for RFC 7231) in #69.

ddeboer added a commit that referenced this pull request Jul 5, 2015
Add a list of default matched status code
@ddeboer ddeboer merged commit a716f6c into FriendsOfSymfony:master Jul 5, 2015
@ddeboer
Copy link
Member

ddeboer commented Jul 5, 2015

Thanks @tgalopin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants