[PLIP-Advisories] Re: [Plone] #9316: Unify folder implementations

plip-advisories at lists.plone.org plip-advisories at lists.plone.org
Thu Oct 1 13:55:15 UTC 2009


#9316: Unify folder implementations
----------------------------+-----------------------------------------------
 Reporter:  smcmahon        |        Owner:  witsch  
     Type:  PLIP            |       Status:  assigned
 Priority:  n/a             |    Milestone:  4.0     
Component:  Infrastructure  |   Resolution:          
 Keywords:                  |  
----------------------------+-----------------------------------------------

Comment(by witsch):

 Replying to [comment:25 davisagli]:
 > (In [29513]) add FWT review, refs #9316

 quoting parts of [browser:buildouts/plone-
 coredev/branches/4.0/plips/plip9316-review-davisagli.txt at 29513 david's
 review] to be able to provide comments inline:


 > Notes and observations [[br]]
 > [[br]]
 > * The original PLIP discussed the creation of a special view optimized
 for
 >   viewing folders with many items.  I presume this was dropped due to
 the
 >   separate PLIP for revamping the collection UI?  That's probably fine.

 well, it wasn't really dropped because of that other PLIP, to be honest.
 it was just that when martin & i started working on this back in
 copenhagen, i never thought of a UI part, and i only read PLIP 191 again
 after steve had already used it as the base for this one... :)

 iow, it was never my intention to provide an update `folder_contents` view
 for this, even though we have of course discussed the implications and
 possible solutions for quite a bit.  we've also created a slightly
 adjusted version in one of our projects, but that's tailored towards the
 partial ordering adapter.

 in any case, i won't have time to come up and implement such a view as
 part of this work.  and i also think that it needs more discussion first.
 sean upton's work (as part of this year's gsoc) seems promising, and i'd
 very much like to see a PLIP for this for 4.1 or 4.2.  it won't
 necessarily me who's driving this, though... :)

 > * The original PLIP also discussed some migration code for converting
 existing
 >   the internal data structures of existing folders.  Does this code
 exist
 >   where where it can be reviewed?

 not yet, it still lives inside a migration package of the project it was
 developed for.  i will merge it into `plone.app.folder` next thing,
 though.

 >   It's crucial that we get this right, of
 >   course, and that it can be made to work for custom subclasses of
 ATFolder
 >   as well.  Assuming this concern is addressed, it looks like custom
 subclasses
 >   will simply pick up the new mixin and "just work" as long as they
 aren't
 >   overriding the old ordering implementation.

 right.  the migration has proven to work for our project: some 15+k
 folders were migrated a while ago and are now used in production by
 several thousand users every (week)day.  so far we haven't seen any
 problems with neither the migration nor the new folder implementation.
 however, i should mention that this project is using the partial ordering
 adapter instead of the default one.

 > * The extension profile in plone.app.folder appears to merely duplicate
 the
 >   change that was made in CMFPlone's Folder.xml.

 right, and the change to the default `Folder.xml` was bad anyway, see
 r30089.  the same commit also keeps the extra profile from being loaded,
 fixing this issue.  you are right, though, the profile is indeed still
 needed for plone 3.x.

 > * bin/alltests shows erroring tests in a bunch of packages, mostly with
 a traceback and error similar to the following:

 this was also fixed by r30089 — the problem was that the factory (in
 `plone.app.folder`) hadn't been properly initialized yet by the time PTC
 was trying to create a folder during site setup.  as `ATFolder` itself is
 now patched (by this PLIP, as opposed to the stand-alone
 `plone.app.folder` for 3.x), the factory can simply remain to be the old
 one.

 > ValueError: Product factory for Folder was invalid

 on a side node, i only realized last night that i left the PLIP in a
 completely unfinished state the night i was trying to wrap it up for the
 first round of reviews.  i found a yet uncommitted change and also
 remembered having seen that error before.  however, i also remember
 constantly dozing off over the keyboard that night — no real surprise at
 3am, i guess...

 at some point i think i gave up on the above mentioned test setup problem,
 deciding i would look into it in the morning.  however, the funny thing to
 me is that apparently i was so tired that i didn't even remember something
 still wasn't quite working the next day.  judging from my time tracking i
 must have thought everything was fine and never looked back...  so again,
 sorry about the mess. :)

 > There are also a lot of test issues in Archetypes.

 right, there are indeed a number of test issues left.  i've just updated
 the branches and checked a couple of packages:

   * `Product.Archetypes` currently has 5 failures, 9 errors (these are
 very likely all because the sample type `SimpleBTreeFolder` can for some
 reason not be adapted to `IOrdering` anymore)
   * `Product.ATContenTypes` has 1 failure in `traversal.txt`
   * `Product.CMFPlone` has 0 failures
   * `plone.folder` has 0 failures (needs to be run using `bin/instance
 test -s plone.folder` for some reason)
   * `plone.app.folder` (which also needs to be run using `bin/instance
 test ...`) has 0 failures, but a problem with importing
 `test_integration.py` (i suspect a circular import or something)

 i will investigate these asap, but none of them should be show-stoppers.
 the same tests all pass on plone 3.x, and i'm pretty sure the failures are
 only triggered because of the integrational changes in `Archetypes` and
 `ATContentTypes`.

 > I assume that these are
 > mostly due to the switch to Plone 4 when this package was developed for
 Plone
 > 3, but come on, these really need to be fixed for this PLIP to be
 considered
 > seriously!

 you're right of course.  sorry about turning this in without properly
 finishing up first.  but like i said, i thought i had... %)

 > * I did not have time to verify the benchmarks included in the PLIP
 overview,
 >   but don't have reason to doubt them.

 actually, it's pretty easy to run them — see the comments in
 [browser:plone.app.folder/trunk/src/plone/app/folder/tests/benchmarks.py at 30097
 benchmark.py].  the [browser:plone.folder/jmeter at 30097 jmeter tests] will
 be trickier to set up, of course.

 > * Any sense of how this affects pickle size and transaction size for
 operations
 >   on different sizes of folders?

 no, i haven't checked yet.  would you have any suggestions on what to test
 here in particular?

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


More information about the PLIP-Advisories mailing list