New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use native strings for ZODB mount points in Python 2 #281

Merged
merged 1 commit into from May 17, 2018

Conversation

Projects
None yet
3 participants
@rbu
Member

rbu commented May 17, 2018

ZODB requires paths and identifiers to be native strings (unicode on
Py3, bytes on Py2). Since the ZConfig parser decodes the config file,
keys and valus in the will consistently be unicode. As a result, the
ZopeDatabase object needs to encode the value back to str/bytes on
Python 2.

@davisagli

This comment has been minimized.

Member

davisagli commented May 17, 2018

Why wasn't this necessary before in Python 2?

@rbu

This comment has been minimized.

Member

rbu commented May 17, 2018

@davisagli, I assume this got broken for the Python 2 case by this commit in ZConfig/loader.py: zopefoundation/ZConfig@010cb2b

Before this commit, Python 2 and 3 would load all of the config as native strings (because they had different code paths). With this commit, both Pythons load the config as unicode StringIO.

@@ -165,6 +166,8 @@ def getName(self):
def computeMountPaths(self):
mps = []
for part in self.config.mount_points:
if PY2 and type(part) == text_type:

This comment has been minimized.

@davisagli

davisagli May 17, 2018

Member

Should probably compare using is

This comment has been minimized.

@rbu

rbu May 17, 2018

Member

Clearly, I should have used isinstance.

Use native strings for ZODB mount points in Python 2
ZODB requires paths and identifiers to be native strings (unicode on
Py3, bytes on Py2). Since the ZConfig parser decodes the config file,
keys and valus in the will consistently be unicode. As a result, the
ZopeDatabase object needs to encode the value back to str/bytes on
Python 2.
@icemac

LGTM.

@icemac

icemac approved these changes May 17, 2018

LGTM.

@rbu rbu merged commit e413288 into master May 17, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls First build on master at 79.186%
Details

@rbu rbu deleted the zodb-mount-points-as-native-strings branch May 17, 2018

@icemac icemac added this to the 4.0b5 milestone May 25, 2018

icemac added a commit that referenced this pull request May 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment