zopefoundation / persistent

automatic persistence for Python objects
https://pypi.org/project/persistent/
Other
46 stars 28 forks source link

TimeStamp __repr__ returns a double quoted string #12

Closed do3cc closed 9 years ago

do3cc commented 9 years ago

The test creates the string the same way so it does not fail. Relevant commits: https://github.com/zopefoundation/persistent/commit/db732a81b0ce4e69bceee1014843b24a21b80fe7 https://github.com/zopefoundation/persistent/commit/e692af8281466fa309aae9273864039dcb287383

and a traceback in the wild where it bit me: https://github.com/ploneintranet/ploneintranet.todo/commit/7f215f69f945b692ae1004f7533c04311c9709fa#commitcomment-8752965

mgedmin commented 9 years ago

I don't understand what the problem is. Current persistent master, on Python 2:

>>> from persistent.TimeStamp import TimeStamp
>>> print repr(TimeStamp(2014, 2, 3))
'\x03\xa4\x8a\xa0\x00\x00\x00\x00'

and on Python 3:

>>> from persistent.TimeStamp import TimeStamp
>>> print(repr(TimeStamp(2014, 2, 3)))
b'\x03\xa4\x8a\xa0\x00\x00\x00\x00'

I don't see any double-quoted strings here.

Also, what does that traceback have to do with this issue? How is __repr__ involved in that ValueError?

(Also, hint: if you edit the comment and wrap the traceback with ..., it'll be more readable.)

do3cc commented 9 years ago

At first I could not reproduce you calling arg, I'll create a different bug report for this. but going back to your example, you print the representation, and it starts with a "'" that is the inner quote. The traceback does not help in understand how this is a problem... The problem occurs more or less here:

https://github.com/zopefoundation/ZODB/blob/master/src/ZODB/utils.py#L147 Just that I am looking at ZODB3 3.10.5 In ZODB3 3.10.5 there is no ts.raw() but a ts

So my problem is caused by a combination of incompatible package versions :-(

do3cc commented 9 years ago

Since ZODB is not a dependency on persistent, there is no way to mark this package incompatible with ZODB 3.10. I close this issue

mgedmin commented 9 years ago

Ah, the return value of __repr__ changed in some version of persistent (but this is not mentioned in the changelog), and that makes older versions of ZODB fail when used with new versions of persistent?

At the very least we should add a changelog notice in persistent, warning people about this (and mention the exact exception this incompatibility causes so it'll come up in Google searches).

(FWIW I don't have the energy to do this.)

tseaver commented 9 years ago

Older versions of ZODB bundle their own version of persistent.

mgedmin commented 9 years ago

Hm. So if the user ends up with a mixture of packages that include an older ZODB and a newer persistent, it's a matter of chance which persistent ends up first on sys.path?

do3cc commented 9 years ago

Yes, I think so

tseaver commented 9 years ago

Just don't install both persistent and ZODB3: persistent should only be used with the separately released ZODB (note the trailing "3" on the "omnibus" release).