Skip to content
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

Extend / Fix the "NDB_PY2_UNPICKLE_COMPAT" flag #112

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kaansoral
Copy link

'latin1' encoding is the fix to most common fix of the DecodeError's, throughout the docs it's the adopted fix as well, which can be seen here: https://cloud.google.com/appengine/docs/standard/python3/services/access

After this fix, I believe the "NDB_PY2_UNPICKLE_COMPAT" flag should be added to the official docs as well

Fixes #111

'latin1' encoding is the fix to most of the common DecodeError's
Copy link

google-cla bot commented Jan 30, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

try:
return pickle.loads(value, encoding='bytes')
except:
return pickle.loads(value, encoding='latin1')
raise
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is encoding='bytes' ever what you would want here anyway? I would maybe try 'latin1' first, it's still going to mess up unicode strings but it's probably better for the common case of datetime objects.

@pnico
Copy link

pnico commented Feb 1, 2024

I filed issue #103 asking basically "what is this thing" and got no responses so good luck ever getting this merged :/

If you don't want to use a modified appengine lib, you could try something like this (YMMV, no guarantees)

try:
    _ = self.data
except UnicodeDecodeError:  # we know there's data, probably stored in Python 2
    try:
        prop = self._properties.get('data')
        self.data = pickle.loads(prop._get_base_value(self).b_val, encoding='latin1')
        # we 'fixed' it, we can use self.data now
    except Exception:
        # couldn't fix it, could raise here or just log something scary and give up:
        self.data = None

@kaansoral
Copy link
Author

Thank you, this was the response to my second issue tracker issue: https://issuetracker.google.com/issues/322838721

I do share the same sentiment with you, I doubt this will be fixed for others too, I suspect we all push the wagon ourselves at this point those who are left

Have no idea what 'bytes' encoding is for, but I kept it in place so that whatever edge case it fixed, is still fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants