[ZODB-Dev] [zopefoundation/ZODB] 49919d: test for POSKeyError during transaction commit

[Jim Fulton]

On Wed, Feb 5, 2014 at 1:57 AM, Marius Gedminas <marius at gedmin.as> wrote:
> On Tue, Feb 04, 2014 at 12:44:09PM -0500, Tres Seaver wrote:
>> On 02/04/2014 06:28 AM, Godefroid Chapelle wrote:
>> > Le 03/02/14 20:53, Tres Seaver a écrit :
>> >> I wish you hadn't pushed that -- some of these changes are definitely
>> >> inappropriate on the 3.10 branch (adding an Acquisition dependency
>> >> is definitely wrong).

Agreed. Note that non-trivial commits to a release branch, like
master, should be
via pull request.

>> >
>> > Acquisition is added as a test dependency. Any hint how to replicate
>> > the bug without acquisition is welcome.
>>
>> Define a subclass of Persistent which emulates what Acquisition does, e.g.:
>>
>>   from persistent import Persistent
>>   class Foo(Persistent):
>>       @property
>>       def _p_jar(self): # or whatever attribute trggers
>>           return object()
>
> What if full replication requires a C extension module?
>
> (I hope that's not true and that it is possible to reproduce the bug
> using some fakes, but I haven't spent the time investigating this.)

I'm going to dig into this.  I'm baffled by the assertion that this has
anything to do with readCurrent.

Regardless of whether it should have been made to the 3.10 branch,
I'm going to use Godefroid's test case to dig further.


>
>> > Which other change is inappropriate ?
>>
>> Adding MANIFEST.in on a release branch seems wrong to me (I don't like
>> them anyway, and we *definitely* don't want to encourage
>> instsall-from-a-github-generated-tarball on a release branch).
>
> That's like objecting if someone adds a .gitignore to a release branch.
> Or a .travis.yml.  It's not code, it's metadata.

Yup.

> (I never liked setuptool's magic "let me query git to see what source
> files you have, but not by default, oh no, instead let's assume
> everybody has installed the non-standard plugin into their system
> Pythons and then let's silently produce broken tarballs if they haven't,
> because obviously implicit is better than explicit, and when there's
> temptation the right thing is to guess" behavior anyway, and we
> *definitely* don't want broken sdists on PyPI.)

I couldn't agree more. One of the advantages of moving to
git was circumventing setuptool's misguided magic.

I've no idea what Tres was referring to wrt
"instsall-from-a-github-generated-tarball", but I use Manifest.in
files in all my modern projects.

Jim

-- 
Jim Fulton
http://www.linkedin.com/in/jimfulton