claudevdm commented on code in PR #35904: URL: https://github.com/apache/beam/pull/35904#discussion_r2350284775
########## 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: Can we do the cloudpickle changes in a separate PR from the dill changes? I would also like us to run the official cloudpickle test suite modified here https://github.com/cloudpipe/cloudpickle/compare/master...claudevdm:cloudpickle:cloudpickle-configurable once this functionality is added to the CloudPickleConfig. Maybe this is a good time to think about the right way to fork cloudpickle with test coverage @tvalentyn ? ########## sdks/python/apache_beam/internal/cloudpickle/cloudpickle.py: ########## @@ -1326,7 +1343,12 @@ def dump(self, obj): raise def __init__( - self, file, protocol=None, buffer_callback=None, config=DEFAULT_CONFIG): + self, + file, + protocol=None, + buffer_callback=None, + config=DEFAULT_CONFIG, + enable_lambda_name=False): Review Comment: +1, can we add this as an option to the CloudPickleConfig, and guard all the new logic in this PR with that config option https://github.com/apache/beam/blob/bde162c5e20543d95ca44230ecda6bb973943150/sdks/python/apache_beam/internal/cloudpickle/cloudpickle.py#L113? -- 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]
