kristynsmith commented on code in PR #35904:
URL: https://github.com/apache/beam/pull/35904#discussion_r2361147373
##########
sdks/python/apache_beam/internal/cloudpickle/cloudpickle.py:
##########
@@ -1266,7 +1274,11 @@ def _dynamic_function_reduce(self, func):
"""Reduce a function that is not pickleable via attribute lookup."""
newargs = self._function_getnewargs(func)
state = _function_getstate(func)
- return (_make_function, newargs, state, None, None, _function_setstate)
+ if type(newargs[0]) == str:
Review Comment:
1. the first element that `function_getnewargs` returns is a string if the
code object identifier was able to be created. in that case, we want to use
make_function_from_identifier.
2. not really sure what you mean here.
##########
sdks/python/apache_beam/internal/cloudpickle/cloudpickle.py:
##########
@@ -1336,6 +1358,7 @@ def __init__(
self.globals_ref = {}
self.proto = int(protocol)
self.config = config
+ self.enable_lambda_name = enable_lambda_name
Review Comment:
my first instinct is `enable_code_identifier_pickling` since that would
match the language used throughout this project. but it's not super clear to
someone completely unfamiliar with this feature. in a docstring i might say
something like "enables the use of a unique, stable code identifier for
pickling" so maybe `enable_stable code_identifier_pickling` would be more clear.
##########
sdks/python/apache_beam/internal/cloudpickle/cloudpickle.py:
##########
@@ -78,6 +78,9 @@
import warnings
import weakref
+from apache_beam.internal.code_object_pickler import get_code_from_identifier
Review Comment:
the only dill changes here are just for handling if the feature is enabled
but that pickling library being used is dill. is that what you're referring to?
this won't be integrated with dill.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
