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

Add ability to register modules to be deeply serialized #417

Merged
merged 74 commits into from Aug 1, 2021
Merged
Show file tree
Hide file tree
Changes from 65 commits
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
8a1578b
TST trigger the bug of #425 in a test
pierreglaser Jun 15, 2021
1de1c4f
FIX remove the __builtins__ from a copy of the module
pierreglaser Jun 15, 2021
05ebfd3
MNT document change inside CHANGES.md
pierreglaser Jun 15, 2021
38364d6
Fixes #206.
Apr 20, 2021
3046111
Adding newline to end of file
Apr 20, 2021
05b0f83
Further flake8 changes
Apr 20, 2021
de1b375
More formatting fixes
Apr 20, 2021
261f127
Renaming function
Jun 14, 2021
9dcb540
Document changes in changelog
Jun 15, 2021
a2ec3a2
Unifying the naming convention
Jun 15, 2021
6e882e8
Updating naming conventions
Jun 15, 2021
cd1fde3
Adding broken module test
Jun 15, 2021
50d56a9
Resolving protocol
Jun 15, 2021
898cf05
Adding instnace check for TypeVar
Jun 15, 2021
65df95b
Merge branch 'master' into 206-deep-serialization
pierreglaser Jun 19, 2021
fd9d250
CLN fix linting errors
pierreglaser Jun 19, 2021
5f84156
fixup! CLN fix linting errors
pierreglaser Jun 19, 2021
14067eb
Making separate typevar chec, and using functions for the `_should_pi…
Jun 21, 2021
cce4708
cosmit
ogrisel Jun 22, 2021
14a5cfe
Update cloudpickle/cloudpickle.py
Samreay Jun 22, 2021
ab57df8
Update tests/cloudpickle_test.py
Samreay Jun 22, 2021
cfcf843
Updating variable names
Jun 22, 2021
cc04c2b
Adding another unit test to check local module patching
Jun 30, 2021
c689adc
Updating names and comments
Jun 30, 2021
bad9f75
I am a savage that didnt run flake8 and I apologise
Jun 30, 2021
23cbfb3
Merge branch 'master' into 206-deep-serialization
ogrisel Jun 30, 2021
261b5a8
Merge branch 'master' into 206-deep-serialization
ogrisel Jun 30, 2021
b29547e
Updating function and variable names
Jul 1, 2021
bd4967f
highlight possible bugs silenced by pytest
pierreglaser Jul 4, 2021
d83e25a
DOC Present the feature in the README
pierreglaser Jul 11, 2021
a7725bd
Removing submodules from arg as it is not directly used
Samreay Jul 11, 2021
c799711
TST rework the test structure
pierreglaser Jul 12, 2021
11eb075
Merge branch 'master' into 206-deep-serialization
pierreglaser Jul 12, 2021
b111a56
TST test by simulating an interactive session
pierreglaser Jul 13, 2021
4a3b3dd
CI debug sys.path issues in CI
pierreglaser Jul 13, 2021
ad85452
CI again
pierreglaser Jul 13, 2021
4f99ad1
CI again
pierreglaser Jul 13, 2021
59a0903
CI again
pierreglaser Jul 13, 2021
0bf7c58
CI again
pierreglaser Jul 13, 2021
43210dc
CI again
pierreglaser Jul 13, 2021
2fdd912
CI again
pierreglaser Jul 13, 2021
03c82cb
CI again
pierreglaser Jul 13, 2021
730c11b
CI again
pierreglaser Jul 13, 2021
1d97bff
CI again
pierreglaser Jul 13, 2021
aa12ac2
CI look only at macos ci builds
pierreglaser Jul 13, 2021
93544c5
CI again
pierreglaser Jul 13, 2021
2d3d1ed
TST: fix isolation procedure on linux
pierreglaser Jul 14, 2021
fd740f2
CI restore testing for all OSes
pierreglaser Jul 14, 2021
1794c2e
TST take into account possisble PYTHONPATH values
pierreglaser Jul 14, 2021
a6637d5
API replace is_registered_... by list_registry
pierreglaser Jul 15, 2021
866f096
CLN remove unused import
pierreglaser Jul 15, 2021
1206827
TST add tests invoving namespace modules and subfolders
pierreglaser Jul 21, 2021
4ee37d9
TST test pickling by value installed packages
pierreglaser Jul 21, 2021
4faafbc
TST add module inside locally importable subfolder
pierreglaser Jul 21, 2021
dae05bb
TST add funcs with globals in _cloudpickle_testpkg
pierreglaser Jul 21, 2021
3d4c896
TST remote co-existence of multiple versions of a func
pierreglaser Jul 21, 2021
4f5942e
TST, FIX some crucial line dissapearing
pierreglaser Jul 21, 2021
c50aa4c
API enforce module-type input for registration api
pierreglaser Jul 22, 2021
5843205
TST (try to) escape backslashes on windows
pierreglaser Jul 22, 2021
5a2b25b
TST (try to) escape backslashes on windows (2)
pierreglaser Jul 22, 2021
2b82f4d
CLN update README after API change
pierreglaser Jul 22, 2021
058c537
TST fix typo in test
pierreglaser Jul 22, 2021
850be6c
CLN remove stale file
pierreglaser Jul 22, 2021
749a6b7
CLN clean up some un-necessary branches
pierreglaser Jul 22, 2021
c5bc41e
postpone relative imports handling to the future
pierreglaser Jul 22, 2021
6ef5ec8
_is_registered_pickle_by_value should take a module
pierreglaser Jul 31, 2021
8004bae
CLN cleaner registration API error messages
pierreglaser Jul 31, 2021
418b848
CLN unused import
pierreglaser Jul 31, 2021
79a38fe
Apply docs suggestions
pierreglaser Jul 31, 2021
4f9af38
Update tests/cloudpickle_test.py
pierreglaser Jul 31, 2021
5f079b0
fix linting errors
pierreglaser Jul 31, 2021
0589dcb
more linting...
pierreglaser Jul 31, 2021
e4ca37d
TST fix a few test mistakes
pierreglaser Jul 31, 2021
1e9a48d
DOC more in-depth context description in README
pierreglaser Jul 31, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGES.md
Expand Up @@ -6,9 +6,13 @@ dev

- Python 3.5 is no longer supported.

- Support for registering modules to be serialised by value. This will
allow for code defined in local modules to be serialised and executed
remotely without those local modules installed on the remote machine.
([PR #417](https://github.com/cloudpipe/cloudpickle/pull/417))

- Fix a side effect altering dynamic modules at pickling time.
([PR #426](https://github.com/cloudpipe/cloudpickle/pull/426))

- Support for pickling type annotations on Python 3.10 as per [PEP 563](
https://www.python.org/dev/peps/pep-0563/)
([PR #400](https://github.com/cloudpipe/cloudpickle/pull/400))
Expand Down
44 changes: 44 additions & 0 deletions README.md
Expand Up @@ -66,6 +66,50 @@ Pickling a function interactively defined in a Python shell session
85
```


Overriding pickle's serialization mechanism for importable constructs:
----------------------------------------------------------------------

An important difference between `cloudpickle` and `pickle` is that
`cloudpickle` can serialize a function or class **by value**, whereas `pickle`
can only serialize it **by reference**, e.g. by serializing its *module
attribute path* (such as `my_module.my_function`).

By default, `cloudpickle` only uses serialization by value in cases where
serialization by reference is usually ineffective, for example when the
function/class to be pickled was constructed in an interactive Python session.

Since `cloudpickle 1.7.0`, it is possible to extend the use of serialization by
value to functions or classes coming from **any pure Python module**. This feature
is useful when the said module is unavailable in the unpickling environment
(making traditional serialization by reference ineffective). To this end,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the motivation (interactive dev on a distributed cluster with long running worker processes) could be made more explicit by reusing the text I suggest in the docstring of register_pickle_by_value above in this review.

`cloudpickle` exposes the
`register_pickle_by_value`/`unregister_pickle_by_value` functions:

```python
>>> import cloudpickle
>>> import my_module
>>> cloudpickle.register_pickle_by_value(my_module)
>>> cloudpickle.dumps(my_module.my_function) # my_function is pickled by value
>>> cloudpickle.unregister_pickle_by_value(my_module)
>>> cloudpickle.dumps(my_module.my_function) # my_function is pickled by reference
```

Note that this feature is still **experimental**, and may fail in the following
situations:

- If the body of a function/class pickled by value contains an `import` statement:
```python
>>> def f():
>>> ... from another_module import g
>>> ... # calling f in the unpickling environment may fail if another_module
>>> ... # is unavailable
>>> ... return g() + 1
```

- If a function pickled by reference uses a function pickled by value during its execution.


Running the tests
-----------------

Expand Down
89 changes: 79 additions & 10 deletions cloudpickle/cloudpickle.py
Expand Up @@ -45,6 +45,7 @@
import builtins
import dis
import opcode
import inspect
import platform
import sys
import types
Expand Down Expand Up @@ -88,6 +89,9 @@ def g():
# communication speed over compatibility:
DEFAULT_PROTOCOL = pickle.HIGHEST_PROTOCOL

# Names of modules whose resources should be treated as dynamic.
_PICKLE_BY_VALUE_MODULES = set()

# Track the provenance of reconstructed dynamic classes to make it possible to
# reconstruct instances from the matching singleton class definition when
# appropriate and preserve the usual "isinstance" semantics of Python objects.
Expand Down Expand Up @@ -124,6 +128,60 @@ def _lookup_class_or_track(class_tracker_id, class_def):
return class_def


def register_pickle_by_value(module):
"""Register that the input module should be pickled by value."""
pierreglaser marked this conversation as resolved.
Show resolved Hide resolved
if not isinstance(module, types.ModuleType):
raise ValueError(
f"Input should be a module object, got {type(module)} instead"
)
# In the future, cloudpickle may need a way to access any module registered
# for pickling by value in order to introspect relative imports inside
# functions pickled by value. (see
# https://github.com/cloudpipe/cloudpickle/pull/417#issuecomment-873684633).
# This access can be ensured by checking that module is present in
# sys.modules at registering time and assuming that it will still be in
# there when accessed during pickling. Another alternative would be to
# store a weakref to the module. Even though cloudpickle does not implement
# this introspection yet, in order to avoid a possible breaking change
# later, we still enforce the presence of module inside sys.modules.
if module.__name__ not in sys.modules:
raise ValueError(
f"{module} was not imported correctly, have you used an "
f"`import` statement to access it?"
)
_PICKLE_BY_VALUE_MODULES.add(module.__name__)


def unregister_pickle_by_value(module):
"""Unregister that the input module should be pickled by value."""
if not isinstance(module, types.ModuleType):
raise ValueError(
f"Input should be a module object, got {type(module)} instead"
pierreglaser marked this conversation as resolved.
Show resolved Hide resolved
)
if module.__name__ not in _PICKLE_BY_VALUE_MODULES:
raise ValueError(f"{module} is not registered for pickle by value")
else:
_PICKLE_BY_VALUE_MODULES.remove(module.__name__)


def list_registry_pickle_by_value():
return _PICKLE_BY_VALUE_MODULES.copy()


def _is_registered_pickle_by_value(module):
module_name = module.__name__ if inspect.ismodule(module) else module
pierreglaser marked this conversation as resolved.
Show resolved Hide resolved
if module_name in _PICKLE_BY_VALUE_MODULES:
return True
while True:
parent_name = module_name.rsplit(".", 1)[0]
if parent_name == module_name:
break
if parent_name in _PICKLE_BY_VALUE_MODULES:
return True
module_name = parent_name
return False


def _whichmodule(obj, name):
"""Find the module an object belongs to.

Expand Down Expand Up @@ -170,18 +228,24 @@ def _whichmodule(obj, name):
return None


def _is_importable(obj, name=None):
"""Dispatcher utility to test the importability of various constructs."""
if isinstance(obj, types.FunctionType):
return _lookup_module_and_qualname(obj, name=name) is not None
elif issubclass(type(obj), type):
return _lookup_module_and_qualname(obj, name=name) is not None
def _should_pickle_by_reference(obj, name=None):
"""Dispatcher utility to test whether an object should be serialised by
value or reference by using importability as a proxy."""
pierreglaser marked this conversation as resolved.
Show resolved Hide resolved
if isinstance(obj, types.FunctionType) or issubclass(type(obj), type):
module_and_name = _lookup_module_and_qualname(obj, name=name)
if module_and_name is None:
return False
module, name = module_and_name
return not _is_registered_pickle_by_value(module.__name__)

elif isinstance(obj, types.ModuleType):
# We assume that sys.modules is primarily used as a cache mechanism for
# the Python import machinery. Checking if a module has been added in
# is sys.modules therefore a cheap and simple heuristic to tell us whether
# we can assume that a given module could be imported by name in
# another Python process.
# is sys.modules therefore a cheap and simple heuristic to tell us
# whether we can assume that a given module could be imported by name
pierreglaser marked this conversation as resolved.
Show resolved Hide resolved
# in another Python process.
if _is_registered_pickle_by_value(obj.__name__):
return False
return obj.__name__ in sys.modules
else:
raise TypeError(
Expand Down Expand Up @@ -839,10 +903,15 @@ def _decompose_typevar(obj):


def _typevar_reduce(obj):
# TypeVar instances have no __qualname__ hence we pass the name explicitly.
# TypeVar instances require the module information hence why we
# are not using the _should_pickle_by_reference directly
module_and_name = _lookup_module_and_qualname(obj, name=obj.__name__)

if module_and_name is None:
return (_make_typevar, _decompose_typevar(obj))
elif _is_registered_pickle_by_value(module_and_name[0].__name__):
return (_make_typevar, _decompose_typevar(obj))

return (getattr, module_and_name)


Expand Down
12 changes: 6 additions & 6 deletions cloudpickle/cloudpickle_fast.py
Expand Up @@ -28,7 +28,7 @@
from .compat import pickle, Pickler
from .cloudpickle import (
_extract_code_globals, _BUILTIN_TYPE_NAMES, DEFAULT_PROTOCOL,
_find_imported_submodules, _get_cell_contents, _is_importable,
_find_imported_submodules, _get_cell_contents, _should_pickle_by_reference,
_builtin_type, _get_or_create_tracker_id, _make_skeleton_class,
_make_skeleton_enum, _extract_class_dict, dynamic_subimport, subimport,
_typevar_reduce, _get_bases, _make_cell, _make_empty_cell, CellType,
Expand Down Expand Up @@ -352,7 +352,7 @@ def _memoryview_reduce(obj):


def _module_reduce(obj):
if _is_importable(obj):
if _should_pickle_by_reference(obj):
return subimport, (obj.__name__,)
else:
# Some external libraries can populate the "__builtins__" entry of a
Expand Down Expand Up @@ -414,7 +414,7 @@ def _class_reduce(obj):
return type, (NotImplemented,)
elif obj in _BUILTIN_TYPE_NAMES:
return _builtin_type, (_BUILTIN_TYPE_NAMES[obj],)
elif not _is_importable(obj):
elif not _should_pickle_by_reference(obj):
return _dynamic_class_reduce(obj)
return NotImplemented

Expand Down Expand Up @@ -559,7 +559,7 @@ def _function_reduce(self, obj):
As opposed to cloudpickle.py, There no special handling for builtin
pypy functions because cloudpickle_fast is CPython-specific.
"""
if _is_importable(obj):
if _should_pickle_by_reference(obj):
return NotImplemented
else:
return self._dynamic_function_reduce(obj)
Expand Down Expand Up @@ -763,7 +763,7 @@ def save_global(self, obj, name=None, pack=struct.pack):
)
elif name is not None:
Pickler.save_global(self, obj, name=name)
elif not _is_importable(obj, name=name):
elif not _should_pickle_by_reference(obj, name=name):
self._save_reduce_pickle5(*_dynamic_class_reduce(obj), obj=obj)
else:
Pickler.save_global(self, obj, name=name)
Expand All @@ -775,7 +775,7 @@ def save_function(self, obj, name=None):
Determines what kind of function obj is (e.g. lambda, defined at
interactive prompt, etc) and handles the pickling appropriately.
"""
if _is_importable(obj, name=name):
if _should_pickle_by_reference(obj, name=name):
return Pickler.save_global(self, obj, name=name)
elif PYPY and isinstance(obj.__code__, builtin_code_type):
return self.save_pypy_builtin_func(obj)
Expand Down