[PLIP-Advisories] Re: [Plone] #7822: Make standard file content types use ZODB BLOB support

plip-advisories at lists.plone.org plip-advisories at lists.plone.org
Wed Sep 30 14:39:56 UTC 2009


#7822: Make standard file content types use ZODB BLOB support
----------------------------+-----------------------------------------------
 Reporter:  limi            |        Owner:  witsch  
     Type:  PLIP            |       Status:  assigned
 Priority:  major           |    Milestone:  4.0     
Component:  Infrastructure  |   Resolution:          
 Keywords:  focusarea       |  
----------------------------+-----------------------------------------------

Comment(by witsch):

 Replying to [comment:42 alecm]:
 > (In [29529]) Add reviw for PLIP #7822 (refs #7822)

 quoting parts of [browser:buildouts/plone-
 coredev/branches/4.0/plips/plip7822-review-alecm.txt at 29529 alec's review]
 to be able to provide comments inline:

 > Users of the product seem to speak favorably of it.

 very nicely put! :)

 > 1) The implementer has failed to include a document describing the
 >    PLIP along with the buildout.

 my bad.  i caught up just now, see r30062.

 > 2) plone.app.blob tests do not pass under Plone 4.0 + Zope 2.12,
 >    though they appear to be well designed and offer reasonable
 >    coverage.

 they pass now, albeit i had to downgrade to ZODB 3.8.3 for the time being
 (please see r29981).

 > 3) The Plone default profile has not been updated to install the BLOB
 >    content types by default.

 fixed in r30057.

 > 4) The BLOB Image type install profile does not appear in the Quick
 >    Installer.

 quick-installer only supports exposes one profile per package.  as image
 support is not quite ready yet for general consumption, and since i'm
 trying to provide a stable add-in for Plone 3.x (i.e. without branching
 things for Plone 4) the "image replacement" profile isn't applied by
 default yet.  once image support is ready, the "default" profile will
 start depending on it.

 for now you'll either have to apply the profile manually via
 `portal_setup` for Plone 3.x — for testing/reviewing in Plone 4.0 it
 should get applied by default (again see r30057)

 > 5) Two BLOB File replacement install profiles appear in portal_setup

 one of those is actually registered for `ITestCasePloneSiteRoot`, but
 still exposed by quick-installer.  i'm not sure if this is intended
 behaviour, but the distinction should be clearer as of r29885.

 > 6) The tests (which again don't pass) depend on
 >    collective.monkeypatcher, but the buildout does not.

 fixed in r29685, thanks martin!

 > 7) The product relies on monkeypatching (ZODB.Blob and
 >    ZPublisher.HTTPRequest).  These patches should be integrated into
 >    the Zope core if we are going to include this product.

 the patch for `setstate_wrapper` can go (as the comments state).  i've
 already made the necessary fix in ZODB, which was subsequently released in
 3.8.1.

 the patches for `HTTPRequest` actually originated in Zope 2.11.  however,
 the original fix doesn't work on windows, and the working version from
 `plone.app.blob` hasn't been pushed upstream yet.  as this is a bug fix,
 though, i don't see what would prevent it from being accepted for 2.12.1
 (also see r19124 for more info).

 > 8) The product depends on plone.app.imaging which monkeypatches
 >    Archetypes.Field.ImageField, this patch should be integrated if
 >    possible.

 yes, it should.  i've added it to the 2dos.


 > Issues and Concerns [[br]]
 >
 > 1) Performance: When serving a small image (600 B) I can get ~108
 >    req/sec with ATFile, but only ~80 req/sec with a BLOB File.

 this was mostly due to a subtle performance issue in
 `archetypes.schemaextender`, which has been fixed in r11344 (in the
 archetypes repo).  with that fix such small files are served at about 100
 req/sec, leaving a performance penalty of less than 10%.

 imho, it's rather unlikely that many small files (i.e. <1k) will be stored
 in actual "File" or "Image" content, but some generated image scales might
 be this small, of course.  possible solutions to this are to either store
 image scales as attributes like before, which can be achieved by simply
 disabling the extra adapter provided by `plone.app.blob`, turning the same
 adapter into a local adapter which can then be enabled separately, or else
 by only storing the payload data on blobs from a certain size, e.g. 16k.

 the last option would probably be best in all cases, but i'm not sure the
 extra effort (and added complexity) is really worth it.  therefore i will
 conduct some more benchmarks and provide the results for the FWT to
 decide.

 > 2) If versioning is enabled for Files, reverting to another version
 >    removes the file content.

 the versioning issues are indeed the ones that worry me most — i've looked
 into resolving them before, but gave up at some point.  i hope that there
 will be a chance to look into this with alec and/or other people in
 budapest.  until then versioning will have to remain disabled for blob-
 based content, which luckily also is the default in Plone.

 > 3) The package depends on archetypes.schemaextender, which is a
 >    worthwhile add-on package in wide use.  However, the package
 >    introduces a significant potential decrease in performance (see
 >    PLIP #9376 and reviews).

 please see above.  the fix in r11344 (in the archetypes repo) resolves the
 performance issues in `archetypes.schemaextender` almost completely.  with
 the fix only the actually added fields will have a performance impact,
 which is — according to my profiling and benchmarks — mostly neglectable,
 though.  please note that this should also render PLIP #9376 mostly
 obsolete.

 > 4) Image scales appear to be generated on the fly and not stored
 >    anywhere.

 this was fixed in r29953.  i will provide updated benchmark results.

 > With some work to properly integrate this package into Plone 4.0 and
 > some further discussion of the potential problems listed above, I
 > would strongly consider recommending this PLIP for inclusion in Plone.

 all remaining issues i consider to be critical before releasing 1.0 (to be
 included into Plone 4.0) are now listed in the [browser:buildouts/plone-
 coredev/branches/4.0/plips/plip7822-blob-support.txt review notes].  there
 are still a number of bug fixes required, especially the one regarding
 versioning, but most of the current test failures in `Plone` and
 `ATContentTypes` are due to the new approach used for image scales (i.e.
 the traversal adapter vs attributes).

-- 
Ticket URL: <http://dev.plone.org/plone/ticket/7822#comment:61>
Plone <http://plone.org>
Plone Content Management System


More information about the PLIP-Advisories mailing list