On Mon, Aug 09, 2010 at 07:37:30PM -0300, Sidnei da Silva wrote:
On Mon, Aug 9, 2010 at 3:25 PM, Marius Gedminas <marius@gedmin.as> wrote:
I've added ETag support for zope.browserresource in a branch: http://zope3.pov.lt/trac/changeset/115596
Does anybody have any comments/objections? If not, I'd like to merge this to trunk and release zope.browserresource 3.11.0.
+1 as long as there's a way to disable or configure how it's computed.
How would you like that to be configured? Option #1: <browser:resource/resourceDirectory etags="off" /> ? Option #2: getMultiAdapter((resource, request), IETag).computeETag() ? This is maybe a bit problematic, because the actual File object that contains all the data--such as filename--doesn't implement any interfaces. And neither does FileResource.
There's some information about issues with ETags here:
http://developer.yahoo.com/performance/rules.html#etags
I see that your implementation uses last-modified + size, which should generally be fine. However if you're load-balancing across two different servers and the timestamps don't match then the ETag is useless.
Would you prefer a sha1 checksum?
On a completely different note, I see that the File object reads the whole file into memory.
And it does that twice for every request that results in a 200 response: once to auto-detect the content-type, the second time to return the actual data.
Hum. Maybe RAM is cheaper than Disk these days and it doesn't matter, but reading whole files into memory generally raises a red flag for me.
hoping-no-one-is-serving-iso-files-through-zope.browserresource-ly yours,
Marius Gedminas -- http://pov.lt/ -- Zope 3/BlueBream consulting and development