Skip to content

Commit

Permalink
[ENG-3970] When normal pickle fails, try dill (#4239)
Browse files Browse the repository at this point in the history
* [ENG-3970] When normal pickle fails, try dill

If dill is not installed, suggest that the user `pip install` it.

Fix #4147

* re-lock depenedencies

* Include original pickle error message for better debugging

When the pickling throws a warning and dill is not installed, include the
original pickle error.

Add a test case for an object that even dill cannot pickle to ensure error path
is hit as expected.

* py3.9 compatibility
  • Loading branch information
masenf authored Oct 24, 2024
1 parent cba6993 commit d85236b
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 7 deletions.
21 changes: 18 additions & 3 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ pytest = ">=7.1.2,<9.0"
pytest-mock = ">=3.10.0,<4.0"
pyright = ">=1.1.229,<1.1.335"
darglint = ">=1.8.1,<2.0"
dill = ">=0.3.8"
toml = ">=0.10.2,<1.0"
pytest-asyncio = ">=0.24.0"
pytest-cov = ">=4.0.0,<6.0"
Expand Down
20 changes: 16 additions & 4 deletions reflex/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -2063,12 +2063,24 @@ def _serialize(self) -> bytes:
"""
try:
return pickle.dumps((self._to_schema(), self))
except pickle.PicklingError:
console.warn(
except (pickle.PicklingError, AttributeError) as og_pickle_error:
error = (
f"Failed to serialize state {self.get_full_name()} due to unpicklable object. "
"This state will not be persisted."
"This state will not be persisted. "
)
return b""
try:
import dill

return dill.dumps((self._to_schema(), self))
except ImportError:
error += (
f"Pickle error: {og_pickle_error}. "
"Consider `pip install 'dill>=0.3.8'` for more exotic serialization support."
)
except (pickle.PicklingError, TypeError, ValueError) as ex:
error += f"Dill was also unable to pickle the state: {ex}"
console.warn(error)
return b""

@classmethod
def _deserialize(
Expand Down
32 changes: 32 additions & 0 deletions tests/units/test_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -3364,3 +3364,35 @@ class Child(State):
assert s.num == 43
c = await root.get_state(Child)
assert c.foo == "bar"


class Obj(Base):
"""A object containing a callable for testing fallback pickle."""

_f: Callable


def test_fallback_pickle():
"""Test that state serialization will fall back to dill."""

class DillState(BaseState):
_o: Optional[Obj] = None
_f: Optional[Callable] = None
_g: Any = None

state = DillState(_reflex_internal_init=True) # type: ignore
state._o = Obj(_f=lambda: 42)
state._f = lambda: 420

pk = state._serialize()

unpickled_state = BaseState._deserialize(pk)
assert unpickled_state._f() == 420
assert unpickled_state._o._f() == 42

# Some object, like generator, are still unpicklable with dill.
state._g = (i for i in range(10))
pk = state._serialize()
assert len(pk) == 0
with pytest.raises(EOFError):
BaseState._deserialize(pk)

0 comments on commit d85236b

Please sign in to comment.