[Erp5-dev] ZODB Components now in master

[Arnaud Fontaine]

Hello,

Vincent Pelletier <vincent at nexedi.com> writes:

> Le   Mon,    12   Mar    2012   11:58:36   +0900,    Arnaud   Fontaine
> <arnaud.fontaine at nexedi.com> a écrit :
>> [0]
>> http://git.erp5.org/gitweb/erp5.git/commitdiff/4406c9b3ccf40861f34ea441299458071bbbfb9c
>> [1]
>> http://git.erp5.org/gitweb/erp5.git/commitdiff/2a7ab878059951e8c0a844d1ce1e1d0df31a1c5f
>
> I'm unsure about the last hunk  of first diff, first commit, and first
> diff of  second commit: so there  is a global dict-ish  holding script
> source code,  which gets flushed on  script edition ?  But  then, it's
> filled from transactional data. If the cache doesn't take transactions
> into  account, it  can contain  stale values.  So we  can access  some
> version of  the script, but maybe  not the one currently  committed it
> there was a timely change to that script.

I agree with  you that this code  could be improved. I will  have a look
when I will have some time. Thanks.

> Side note: in  my understanding, the only grave bugs  we can have when
> moving  code to  ZODB are  transactional/concurrency issues.  They are
> most certainly  not covered by  existing test suites (because  no need
> until now,  and it's notoriously difficult  to test). If this  kind of
> bug is not  properly understood, and code is not  carefully written to
> avoid them,  I am very scared  of this new feature.  I understand that
> "what  PDB/traceback display  when an  error  is thrown"  is not  *so*
> critical, but I have not reviewed other patches and hence fear to find
> the same shortcomings there.

>From  a threads  point of  view,  resetting and  loading Components  are
protected  by  a  reentrant  lock (Base.aq_method_lock,  also  used  for
loading Portal  Type classes) to  avoid concurrency issues,  therefore I
think it must  be safe from this  point of view, and of  course the code
was written with that in mind.

Concerning synchronisation  between ZEO clients, it  is achieved through
the same mechanism as Portal Type class, namely a cache cookie.

Of course, feel free to review the patches and point out possible issues
as it's  always better  to get as  much reviews as  possible as  well as
avoiding to spread  FUD ;-).  Especially considering that  the core code
is not so huge:

git diff 6b765217..5be5084a -- product/ERP5Type product/ERP5/ERP5Site.py

> Also, reading  linecache doc (as  of 2.7), "cache" module  property is
> not  documented.  Can  we  really  rely on  it  ?  In  my  experience,
> undocumented behaviours tend  to be often broken in 2.7  (maybe to get
> closer to python 3). We had at least 2 such breakages triggered in NEO
> by *minor* python updates.

Well, the only supported version for Zope 2.12 is Python 2.6, so I don't
see any problem with that. Perhaps  NEO does not the same requirement as
it's supposed to work on any Python version?

Also, not monkey-patching linecache would introduce much more problem as
it would require to copy and paste a lot of code from pdb module because
it  was  not  designed  at  all to  cope  with  non-filesystem  modules.
Therefore, despite  linecache not being  documented, I think  that's the
best way  as it requires  only a small monkey  patch which can  still be
updated when we  will moved to a later version  of Zope requiring Python
2.7.

Regards,
-- 
Arnaud Fontaine