[Zope3-dev] opinion requested from people working towards therelease: doctest inaccuracy for zope.app.form.utility

Gary Poster gary at zope.com
Mon Mar 7 15:09:58 EST 2005


On Mar 6, 2005, at 7:36 AM, Garrett Smith wrote:

> Rob Jeschofnik wrote:
>> Gary Poster wrote:
>>
>>> [...]
>>> (a) check this in,
>>> (b) change the XXX to a TODO and check it in,
>>> (c) change the test description to actually describe the current
>>> behavior, or (d) try to change the code to match the description?
>>>
>>> If (d) is your response, then please also give me a fallback option
>>> (a, b, or c) if I don't have time.  As I write in my XXX, I have a
>>> workaround already.
>>
>> My choice would be option (d) - It really does seem like an error to
>> me to accept part of the input of a form that contains some invalid
>> items.. how can you gurantee any logic that uses these values is
>> correct when there are some invalid inputs? (or have I misunderstood
>> the problem at hand?)
>
> Heres the current behavior:
>
> - applyWidgetsChanges will successfully apply any widget changes that 
> it
> can; if some fail because of errors, the errors are provided in an
> exception; the changes that succeed 'stick'
>
> - the form, noting one or more errors, calls get_transaction().abort(),
> to ensure that any changes to the ZODB (or other transactionally 
> managed
> data sources) are rolledback
>
> I don't think we want to build transactional support into
> applyWidgetsChanges -- it's going to start getting complex and I get
> there will be more errors for that approach than using
> get_transaction().abort(). I don't see any benefit since we'd have to
> abort the transaction anyway to accommodate any side effects of 
> updating
> the object.
>
> One could argue that this is "brute force", but I'd say it's worth it 
> to
> keep the forms machinery free of transactional logic.

Yes, Zope relies on transactions, and this code lives in zope.app.  If 
we ever pushed these utilities out of zope.app into zope (and we 
might), I might be more inclined to make them less reliant on 
transaction rollbacks.  For now, this is a reasonable plan.

>> As for a backup plan... if you didn't have time for it I'd suggest (e)
>> leave it as an XXX and raise it as a Collector item, so someone else
>> fixes it. I don't really like the idea of changing tests to match
>> dodgy code.

FWIW, that was (a).  The collector issue is an important nice-to-have, 
the XXX a required marker.

Gary



More information about the Zope3-dev mailing list