Add support for PyPy, Python 2.7, and Python 3. #1

jamadden · 2015-04-06T17:00:20Z

This PR adds support for versions of Python > 2.6 and PyPy by using zodbpickle when needed.

As summarized in the zodbpickle issue, noload is broken in CPython 2.7, as well as being completely missing in PyPy and Python 3. zodbpickle fixes this for CPython 2.7 and adds it back for PyPy and Python 3.

The tests were initially broken under 2.6 due to changes in the latest ZODB/persistent, so the first/fourth commit fixes those tests. Then CPython 2.7 support was added by creating a dependency on zodbpickle and using it when needed. Fixing PyPy support was a matter of ensuring that resources get closed in a timely fashion, and Python 3 support consisted of some minor syntax changes to account for the print statement and the differences between bytes and strings, plus one __bool__ alias.

A tox.ini for local use and a .travis.yml file for github use were added to ensure the tests run on all supported platforms (this required a MANIFEST.in for tox):

$ tox
.....
____________________ summary __________________________
  py26: commands succeeded
  py27: commands succeeded
  py32: commands succeeded
  py33: commands succeeded
  py34: commands succeeded
  pypy: commands succeeded

The smelliest part is that a monkey-patch against ZODB.serialize is necessary to work with older versions of ZODB that don't properly support PyPy (since it has never had a version of noload). I can submit a PR for ZODB to fix that problem going forward.

There are two changes. First, ZODB has changed the way that blob POSKeyErrors are represented. Second, objects 3 and 4 that really are garbage from db2 are collected during the first pass. This has a small trickle down effect.

…ed in tox.ini and .travis.yml.

…t) is to use the 'raw' method of the timestamp, not try to get its repr. The repr of a persistent timestamp changed sometime ago.

mgedmin · 2015-04-07T05:59:48Z

.travis.yml

+    - 3.3
+    - 3.4
+install:
+    - pip install . --use-mirrors


--use-mirrors is deprecated and pip ignores it.

This is also an excellent opportunity to update this PR so we can see if Travis will start building it.

mgedmin · 2015-04-07T06:19:56Z

I tried to run the tox tests locally, and I'm seeing pypy3 failures:

Error locking file 1.fs.lock; pid=13911
Traceback (most recent call last):
  File "/home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/zc/lockfile/__init__.py", line 57, in _lock_file
    fcntl.flock(file.fileno(), _flags)
IOError: [Errno 11] Resource temporarily unavailable

Full log at https://gist.github.com/mgedmin/56b7efa581fbfb206dc8

See #2.

mgedmin · 2015-04-07T06:25:22Z

Apologies for introducing a merge conflict: I didn't want to leave Travis CI enabled without a .travis.yml in master (beause without a .travis.yml Travis defaults to Ruby builds, which are useless for us).

Would you mind rebasing?

…v qualifier, that's long released). Explicitly note all the Python 3 versions tested/supported---previously it was just a copy of Acquisition's classifiers which only list the latest.

jamadden · 2015-04-07T12:24:53Z

Good point about pypy3. It wasn't on my path so tox was happily ignoring it :(

It could be a GC/del issue in zc.lockfile (or its caller?) because of the ResourceWarnings on the files that it attempts to lock. Then again, PyPy3 is a major release behind PyPy2 so there could be an issue in PyPy. For now, I think I'll comment out pypy3 from tox; it wasn't in .travis.yml anyway.

There are two changes. First, ZODB has changed the way that blob POSKeyErrors are represented. Second, objects 3 and 4 that really are garbage from db2 are collected during the first pass. This has a small trickle down effect.

…ed in tox.ini and .travis.yml.

…t) is to use the 'raw' method of the timestamp, not try to get its repr. The repr of a persistent timestamp changed sometime ago.

…v qualifier, that's long released). Explicitly note all the Python 3 versions tested/supported---previously it was just a copy of Acquisition's classifiers which only list the latest.

jamadden · 2015-04-07T12:30:11Z

Rebased, and the Travis build ran clean. Thank you!

mgedmin · 2015-04-07T13:47:59Z

LGTM, but I'm not familiar with zc.zodbdgc (and I'm not willing to take responsibility for it ;-), so I'll wait for someone else to review and merge this.

(If the PR sits ignored for a week or two, I think it would be fine to merge it yourself -- I see you're a ZF member. You'll need to get @jimfulton's attention to push a new release to PyPI, though, he's the only one with PyPI access to zc.zodbdgc.)

…es the use of _p_changed.

…ckfile issues from earlier. Maybe someone else can?

…py will?

jamadden · 2015-04-15T12:29:17Z

@mgedmin If you have a chance, could you please try the latest code with tox -e pypy3? I'm not able to reproduce the zc.lockfile issue anymore. I tried to add it to .travis.yml, but travis isn't kicking off for some reason.

jamadden · 2015-04-15T12:35:07Z

@mgedmin Whups, there went Travis. Build was green with PyPy3.

mgedmin · 2015-04-15T13:02:25Z

tox -e pypy3 of commit 666772f: green.

There were quite a few ResourceWarning: unclosed file ... warnings. I don't know if you care about those:

/home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/BTrees/fsBTree.py:59: ResourceWarning: unclosed file <_io.BufferedRandom name=21>
  while keys and values:
/home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/BTrees/fsBTree.py:59: ResourceWarning: unclosed file <_io.BufferedRandom name=22>
  while keys and values:
/home/mg/src/zopefoundation/zc.zodbdgc/src/zc/zodbdgc/__init__.py:366: ResourceWarning: unclosed file <_io.BufferedRandom name=23>
  data = self[name][oid[:6]]
/home/mg/src/zopefoundation/zc.zodbdgc/src/zc/zodbdgc/__init__.py:366: ResourceWarning: unclosed file <_io.BufferedRandom name=21>
  data = self[name][oid[:6]]
/home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/zodbpickle/pickle_3.py:867: ResourceWarning: unclosed file <_io.BufferedRandom name=36>
  key = read(1)
/home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/zodbpickle/pickle_3.py:867: ResourceWarning: unclosed file <_io.BufferedRandom name=34>
  key = read(1)
./home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/persistent/persistence.py:246: ResourceWarning: unclosed file <_io.BufferedRandom name=20>
  if _OGA(self, '_Persistent__flags') is None:
/home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/persistent/persistence.py:246: ResourceWarning: unclosed file <_io.BufferedRandom name=33>
  if _OGA(self, '_Persistent__flags') is None:
/home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/persistent/persistence.py:246: ResourceWarning: unclosed file <_io.BufferedRandom name=12>
  if _OGA(self, '_Persistent__flags') is None:
/home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/persistent/persistence.py:246: ResourceWarning: unclosed file <_io.BufferedReader name='/tmp/tmp_g27am/one.fs'>
  if _OGA(self, '_Persistent__flags') is None:
/home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/persistent/persistence.py:246: ResourceWarning: unclosed file <_io.BufferedRandom name=14>
  if _OGA(self, '_Persistent__flags') is None:
/home/mg/src/zopefoundation/zc.zodbdgc/.tox/pypy3/site-packages/persistent/persistence.py:246: ResourceWarning: unclosed file <_io.BufferedRandom name=19>
  if _OGA(self, '_Persistent__flags') is None:

BTW python setup.py -q test is better than python setup.py test -q.

jamadden · 2015-04-15T13:11:02Z

I do care about the ResourceWarnings, but I haven't been able to track them down. The lines in question are different on every run, e.g.,

//zc.zodbdgc/.tox/pypy3/lib-python/3/encodings/utf_8.py:16: ResourceWarning: unclosed file <_io.BufferedRandom name=30>
  return codecs.utf_8_decode(input, errors, True)
//zc.zodbdgc/.tox/pypy3/lib-python/3/encodings/utf_8.py:16: ResourceWarning: unclosed file <_io.BufferedRandom name=31>
  return codecs.utf_8_decode(input, errors, True)
//zc.zodbdgc/.tox/pypy3/lib-python/3/encodings/utf_8.py:16: ResourceWarning: unclosed file <_io.BufferedRandom name=32>
  return codecs.utf_8_decode(input, errors, True)
//zc.zodbdgc/.tox/pypy3/lib-python/3/encodings/utf_8.py:16: ResourceWarning: unclosed file <_io.BufferedRandom name=33>
  return codecs.utf_8_decode(input, errors, True)
//zc.zodbdgc/.tox/pypy3/site-packages/BTrees/_base.py:562: ResourceWarning: unclosed file <_io.BufferedRandom name=47>

The code in question doesn't use the io package, it just uses normal files, which under Python 3 are <_io.BufferedReader> or <_io.TextIOWrapper>, not BufferedRandom. My suspicion is that these have something to do with the doctest machinery. I'll spend some time trying to confirm that.

Thanks for the tip for tox.ini, I just copied that section from some other project without thinking.

…rceWarning/leak under Py3/PyPy. However, the bulk of the remainder are due to the use of a TemporaryFile in the return value from gc_command, which closing could potentially break semantics. Needs input.

jamadden · 2015-04-15T15:03:55Z

@mgedmin I was looking in the wrong place. Under the covers, tempfile.TemporaryFile uses _io.BufferedRandom. It turns out the return value of the gc_command method returns a Bad object that has such a temporary file open.

Now, the test cases all continue to pass if that temporary file is closed (thus solving the ResourceWarning/leak): they just want to iterate over it, which is all handled in memory. However, the Bad object also supports a method iterator(name=) method which does require access to the temporary file. This is not documented or tested. Then again, neither is the return value of gc_command (other than the simple iteration case), so no users would know that they need to call close on the return value in order to avoid a ResourceWarning.

I can see a few options:

Do nothing. Live with the ResourceWarnings/leaks, but don't break anybody that's using the undocumented features of Bad.
Document the features of Bad, including calling close. Add test cases for this and update the existing cases to properly close the object. This is more work, but has the advantage of not breaking any callers.
Make the on-disk behaviour of Bad optional, defaulting to an in-memory version only. This has the advantage of not requiring test changes or additions (to get the same level of coverage as current), plus it makes the GC process potentially faster by not requiring I/O. The disadvantage, aside from the implementation work, is that those (few?) callers that use the undocumented on-disk features will require a change.
Remove the undocumented on-disk features of Bad. Solves the leaks, improves test coverage, and GC speed. Breaks callers relying on them.

Doing nothing may be the easiest way to get the PR completed and merged, although option 4 is close :) @jimfulton no doubt has the best view of the original intent and use cases, hopefully he will have some time to weigh in.

mgedmin · 2015-04-15T15:13:55Z

+1 for not tackling ResourceWarnings in this PR.

jamadden · 2015-08-14T15:53:56Z

Bump. Unless there are objections, per the recommendation earlier of @mgemin I will probably merge this soon (my organization is about to try to move to pypy)

Add support for PyPy, Python 2.7, and Python 3.

jamadden added 4 commits April 6, 2015 08:57

Make tests pass under Python 2.6 with latest ZODB.

15a639d

There are two changes. First, ZODB has changed the way that blob POSKeyErrors are represented. Second, objects 3 and 4 that really are garbage from db2 are collected during the first pass. This has a small trickle down effect.

Make tests pass under CPython2.7 by using zodbpickle.

6485058

Fix tests under PyPy and add support for Python3. Ensure this is test…

e05e24b

…ed in tox.ini and .travis.yml.

The proper fix for the tests under Python 2.6 (and newer, it turns ou…

90ca1dc

…t) is to use the 'raw' method of the timestamp, not try to get its repr. The repr of a persistent timestamp changed sometime ago.

jamadden mentioned this pull request Apr 6, 2015

Enable Travis CI support #2

Closed

Note support for Python 2.7 in changelog.

cff0f45

mgedmin reviewed Apr 7, 2015
View reviewed changes

Add a .travis.yml

8fb35e5

See #2.

jamadden and others added 4 commits April 7, 2015 06:49

Move changelog entry to existing README.txt.

f8ea269

Merge .travis.yml from master in 8fb35e5

2c90a9e

Add version pins to deps that match the ones from ZODB3 (minus the de…

01574a1

…v qualifier, that's long released). Explicitly note all the Python 3 versions tested/supported---previously it was just a copy of Acquisition's classifiers which only list the latest.

PEP8 spacing

2fc9293

jamadden added 9 commits April 7, 2015 07:24

Comment out pypy3 from tox.ini; see comments in that file.

48026dc

Make tests pass under Python 2.6 with latest ZODB.

19306f8

There are two changes. First, ZODB has changed the way that blob POSKeyErrors are represented. Second, objects 3 and 4 that really are garbage from db2 are collected during the first pass. This has a small trickle down effect.

Make tests pass under CPython2.7 by using zodbpickle.

5f310e9

Fix tests under PyPy and add support for Python3. Ensure this is test…

2abc7f2

…ed in tox.ini and .travis.yml.

The proper fix for the tests under Python 2.6 (and newer, it turns ou…

2259ff5

…t) is to use the 'raw' method of the timestamp, not try to get its repr. The repr of a persistent timestamp changed sometime ago.

Note support for Python 2.7 in changelog.

310c240

Move changelog entry to existing README.txt.

2fc38b0

Add version pins to deps that match the ones from ZODB3 (minus the de…

1ff22d7

…v qualifier, that's long released). Explicitly note all the Python 3 versions tested/supported---previously it was just a copy of Acquisition's classifiers which only list the latest.

PEP8 spacing

283dd87

jamadden added 2 commits April 7, 2015 07:26

Comment out pypy3 from tox.ini; see comments in that file.

d87dd3f

Rebasing.

7b8b309

Correct comment: we observed the BTree problem in OOBtreePy

d760462

jamadden mentioned this pull request Apr 7, 2015

Fix setting _p_changed for small pure-Python BTrees. zopefoundation/BTrees#11

Merged

jamadden added 4 commits April 7, 2015 10:49

Update comment to point to zopefoundation/BTrees#11 issue that requir…

fc180b0

…es the use of _p_changed.

Probably helpful to explain *how* the test fails.

03c058b

Add pypy3 to tox.ini and travis.yml; I'm not able to reproduce the lo…

b1a6e22

…ckfile issues from earlier. Maybe someone else can?

That last commit didn't kick off travis; maybe a tiny tweak to setup.…

666772f

…py will?

Be much more careful about closing opened files. This fixes one Resou…

c97bbc2

…rceWarning/leak under Py3/PyPy. However, the bulk of the remainder are due to the use of a TemporaryFile in the return value from gc_command, which closing could potentially break semantics. Needs input.

jamadden added a commit that referenced this pull request Aug 20, 2015

Merge pull request #1 from NextThought/with-zodbpickle

778966c

Add support for PyPy, Python 2.7, and Python 3.

jamadden merged commit 778966c into zopefoundation:master Aug 20, 2015

jamadden deleted the with-zodbpickle branch August 20, 2015 14:53

jamadden mentioned this pull request Aug 25, 2015

Release? #3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for PyPy, Python 2.7, and Python 3. #1

Add support for PyPy, Python 2.7, and Python 3. #1

jamadden commented Apr 6, 2015

mgedmin Apr 7, 2015

mgedmin commented Apr 7, 2015

mgedmin commented Apr 7, 2015

jamadden commented Apr 7, 2015

jamadden commented Apr 7, 2015

mgedmin commented Apr 7, 2015

jamadden commented Apr 15, 2015

jamadden commented Apr 15, 2015

mgedmin commented Apr 15, 2015

jamadden commented Apr 15, 2015

jamadden commented Apr 15, 2015

mgedmin commented Apr 15, 2015

jamadden commented Aug 14, 2015

Add support for PyPy, Python 2.7, and Python 3. #1

Add support for PyPy, Python 2.7, and Python 3. #1

Conversation

jamadden commented Apr 6, 2015

mgedmin Apr 7, 2015

Choose a reason for hiding this comment

mgedmin commented Apr 7, 2015

mgedmin commented Apr 7, 2015

jamadden commented Apr 7, 2015

jamadden commented Apr 7, 2015

mgedmin commented Apr 7, 2015

jamadden commented Apr 15, 2015

jamadden commented Apr 15, 2015

mgedmin commented Apr 15, 2015

jamadden commented Apr 15, 2015

jamadden commented Apr 15, 2015

mgedmin commented Apr 15, 2015

jamadden commented Aug 14, 2015