Browse Source

ref: remove patching of pickle dumping (#67241)

python 2 is long in the past and we are no longer using pickle in the
data pipeline after changing all postgres data to json

<!-- Describe your PR here. -->
anthony sottile 11 months ago
parent
commit
79bf78bb18
2 changed files with 0 additions and 122 deletions
  1. 0 93
      src/sentry/monkey/pickle.py
  2. 0 29
      tests/sentry/utils/test_pickle_protocol.py

+ 0 - 93
src/sentry/monkey/pickle.py

@@ -45,53 +45,14 @@
 #    contains a datetime object, or non-ASCII str data, it will fail with a
 #    UnicodeDecodeError, in which case we will decode strings as latin-1.
 #
-#  - At the moment we DO NOT patch `pickle.load`, since it may or may not be
-#    the case that we can seek to the start of the passed file-like object. If
-#    we do have usages of it, we'll have to patch them specifically based on
-#    how the file is passed.
-#
 # [0]: https://rebeccabilbro.github.io/convert-py2-pickles-to-py3/#python-2-objects-vs-python-3-objects
 
 
 def patch_pickle_loaders():
     import pickle
 
-    # TODO(python3): We use the pickles `2` protocol as it is supported in 2 and 3.
-    #
-    #  - python3 defaults to a protocol > 2 (depending on the version, see [0]).
-    #  - python2 defaults to protocol 2.
-    #
-    # This is ONLY required for the transition of Python 2 -> 3. There will be
-    # a brief period where data may be pickled in python3 (during deploy, or if
-    # we rollback), where if we did not declare the version, would be in format
-    # that python 2's pickle COULD NOT decode.
-    #
-    # Once the python3 transition is complete we can use a higher version
-    #
-    # NOTE: from the documentation:
-    #       > The protocol version of the pickle is detected automatically
-    #
-    # [0]: https://docs.python.org/3/library/pickle.html#pickle-protocols
-    #
-    # XXX(epurkhiser): Unfortunately changing this module property is NOT
-    # enough. Python 3 will use _pickle (aka new cpickle) if it is available
-    # (which it usually will be). In this case it will NOT read from
-    # DEFAULT_PROTOCOL, as the module functions passthrough to the C
-    # implementation, which does not have a mutable DEFAULT_PROTOCOL module
-    # property.
-    #
-    # I'm primarily leaving this here for consistency and documentation
-    #
-    # XXX(epurkhiser): BIG IMPORTANT NOTE! When changing this, we will have to
-    # make some updates to our data pipeline, which currently uses 'pickle.js'
-    # to depickle some data using javascript.
-    pickle.DEFAULT_PROTOCOL = 2
-
     original_pickle_load = pickle.load
-    original_pickle_dump = pickle.dump
     original_pickle_loads = pickle.loads
-    original_pickle_dumps = pickle.dumps
-    original_pickle_Pickler = pickle.Pickler
     original_pickle_Unpickler = pickle.Unpickler
 
     # Patched Picker and Unpickler
@@ -100,25 +61,6 @@ def patch_pickle_loaders():
     # C module we can't subclass, so instead we just delegate with __getattr__.
     # It's very possible we missed some more subtle uses of the classes here.
 
-    class CompatPickler:
-        def __init__(self, *args, **kwargs):
-            # If we don't explicitly pass in a protocol, use DEFAULT_PROTOCOL
-            # Enforce protocol kwarg as DEFAULT_PROTOCOL. See the comment above
-            # DEFAULT_PROTOCOL above to understand why we must pass the kwarg due
-            # to _pickle.
-            if len(args) == 1:
-                if not kwargs.get("protocol"):
-                    kwargs["protocol"] = pickle.DEFAULT_PROTOCOL
-            else:
-                largs = list(args)
-                largs[1] = pickle.DEFAULT_PROTOCOL
-                args = tuple(largs)
-
-            self.__pickler = original_pickle_Pickler(*args, **kwargs)
-
-        def __getattr__(self, key):
-            return getattr(self.__pickler, key)
-
     class CompatUnpickler:
         def __init__(self, *args, **kwargs):
             self.__orig_args = args
@@ -151,38 +93,6 @@ def patch_pickle_loaders():
                 self.__make_unpickler()
                 return self.__unpickler.load()
 
-    # Patched dump and dumps
-
-    def py3_compat_pickle_dump(*args, **kwargs):
-        # If we don't explicitly pass in a protocol, use DEFAULT_PROTOCOL
-        # Enforce protocol kwarg as DEFAULT_PROTOCOL. See the comment above
-        # DEFAULT_PROTOCOL above to understand why we must pass the kwarg due
-        # to _pickle.
-        if len(args) == 1:
-            if not kwargs.get("protocol"):
-                kwargs["protocol"] = pickle.DEFAULT_PROTOCOL
-        else:
-            largs = list(args)
-            largs[1] = pickle.DEFAULT_PROTOCOL
-            args = tuple(largs)
-
-        return original_pickle_dump(*args, **kwargs)
-
-    def py3_compat_pickle_dumps(*args, **kwargs):
-        # If we don't explicitly pass in a protocol, use DEFAULT_PROTOCOL
-        # Enforce protocol kwarg as DEFAULT_PROTOCOL. See the comment above
-        # DEFAULT_PROTOCOL above to understand why we must pass the kwarg due
-        # to _pickle.
-        if len(args) == 1:
-            if not kwargs.get("protocol"):
-                kwargs["protocol"] = pickle.DEFAULT_PROTOCOL
-        else:
-            largs = list(args)
-            largs[1] = pickle.DEFAULT_PROTOCOL
-            args = tuple(largs)
-
-        return original_pickle_dumps(*args, **kwargs)
-
     # Patched load and loads
 
     def py3_compat_pickle_load(*args, **kwargs):
@@ -208,8 +118,5 @@ def patch_pickle_loaders():
             return original_pickle_loads(*args, **kwargs)
 
     pickle.load = py3_compat_pickle_load
-    pickle.dump = py3_compat_pickle_dump
     pickle.loads = py3_compat_pickle_loads
-    pickle.dumps = py3_compat_pickle_dumps
-    pickle.Pickler = CompatPickler
     pickle.Unpickler = CompatUnpickler

+ 0 - 29
tests/sentry/utils/test_pickle_protocol.py

@@ -1,29 +0,0 @@
-import pickle
-from pickle import PickleBuffer, PickleError
-
-import pytest
-
-from sentry.testutils.cases import TestCase
-
-
-class PickleProtocolTestCase(TestCase):
-    """
-    At the time of adding this test we still monkey patch `pickle` and hardcode the protocol to be 2.
-    For legacy reasons see `src/sentry/monkey/pickle.py`.
-
-    This test is for a change that's being made to allow explicitly passing a newer protocol to
-    pickle. If we remove the monkey patching to pickle there is no longer a need for this test.
-
-    """
-
-    def test_pickle_protocol(self):
-        data = b"iamsomedata"
-
-        pickled_data = PickleBuffer(data)
-        with pytest.raises(PickleError) as excinfo:
-            result = pickle.dumps(pickled_data)
-
-        assert "PickleBuffer can only pickled with protocol >= 5" == str(excinfo.value)
-
-        result = pickle.dumps(pickled_data, protocol=5)
-        assert result