[Twisted-Python] Re: [Twisted-commits] r15860 - Move checkPreconditions() from web2.dav.static.DAVFile to web2.resource.RenderMixIn.
On Feb 8, 2006, at 7:49 PM, Wilfredo Sanchez wrote:
Move checkPreconditions() from web2.dav.static.DAVFile to web2.resource.RenderMixIn. Add test suite (ported foom's tests for server.checkpreconditions()). Fix some bugs found by the tests. (Holy cow, checkpreconditions are a PITA.)
I don't like this part. You've made the precondition check code much longer, IMO harder to understand (figuring out how the return values of one method influenced the rest of the execution is not easy), and pinned it to being part of Resource, rather than being reusable. I'm not sure of the need, given that the previous implementation was callable from Resource as it was. Why not just use it... Nextly, I'm somewhat concerned about the change to calling the precondition check before GET rendering instead of afterwards: I think your use cases are probably not typical. I believe most people will write resources which generate dynamic content, and if they're _really good_ about it, they'll simultaneously generate an etag and a last-modified time. This is also the only way that such information can come out of wsgi, cgi, proxies to other servers, etc. The way things were set up before made Not Modified responses just "magically" show up correctly when you set those headers in your response. No more interface was required than that, which means that the response could come from anywhere. Now, instead, you force the author to implement resource.etag() and resource.lastModified() in order to get that behavior. It's really easy for file-like resources to do so, but not so much so for most other kinds. And the thing is, it's OK to check afterwards for the kinds of requests a web browser makes: GET/HEAD/POST. GET/HEAD don't change things so it's acceptable to do their check at any time, and while POST does change things, as a practical matter nobody actually sends conditionals on post requests, because what would it be useful for? (and nothing would be set up to handle such a thing). However, I'm currently ignoring conditionals on PostableResource's http_POST, which while okay as far as real-world interoperability goes, it is certainly not _right_. And to make things worse, the post- processing filter doesn't confine itself to checking only GET/HEAD requests, so the client will get back a precondition failed message, even though the method was executed. At the very least a POST with an If-Unmodified-Since/If-Match should return precondition failed. If there was code on Resource to handle that automatically, I could not have forgetten to do it. So, um, I dunno what to conclude. :) James
Sorry, I meant to send this to twisted-web instead. Plz make any reply there, instead. James On Feb 9, 2006, at 1:34 PM, James Y Knight wrote:
On Feb 8, 2006, at 7:49 PM, Wilfredo Sanchez wrote:
Move checkPreconditions() from web2.dav.static.DAVFile to web2.resource.RenderMixIn. Add test suite (ported foom's tests for server.checkpreconditions()). Fix some bugs found by the tests. (Holy cow, checkpreconditions are a PITA.)
I don't like this part. You've made the precondition check code much longer, IMO harder to understand (figuring out how the return values of one method influenced the rest of the execution is not easy), and pinned it to being part of Resource, rather than being reusable. I'm not sure of the need, given that the previous implementation was callable from Resource as it was. Why not just use it...
Nextly, I'm somewhat concerned about the change to calling the precondition check before GET rendering instead of afterwards: I think your use cases are probably not typical. I believe most people will write resources which generate dynamic content, and if they're _really good_ about it, they'll simultaneously generate an etag and a last-modified time. This is also the only way that such information can come out of wsgi, cgi, proxies to other servers, etc.
The way things were set up before made Not Modified responses just "magically" show up correctly when you set those headers in your response. No more interface was required than that, which means that the response could come from anywhere. Now, instead, you force the author to implement resource.etag() and resource.lastModified() in order to get that behavior. It's really easy for file-like resources to do so, but not so much so for most other kinds.
And the thing is, it's OK to check afterwards for the kinds of requests a web browser makes: GET/HEAD/POST. GET/HEAD don't change things so it's acceptable to do their check at any time, and while POST does change things, as a practical matter nobody actually sends conditionals on post requests, because what would it be useful for? (and nothing would be set up to handle such a thing).
However, I'm currently ignoring conditionals on PostableResource's http_POST, which while okay as far as real-world interoperability goes, it is certainly not _right_. And to make things worse, the post-processing filter doesn't confine itself to checking only GET/ HEAD requests, so the client will get back a precondition failed message, even though the method was executed. At the very least a POST with an If-Unmodified-Since/If-Match should return precondition failed. If there was code on Resource to handle that automatically, I could not have forgetten to do it.
So, um, I dunno what to conclude. :)
James
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
participants (1)
-
James Y Knight