Abacn commented on code in PR #36271: URL: https://github.com/apache/beam/pull/36271#discussion_r2392483059
########## sdks/python/apache_beam/internal/cloudpickle_pickler.py: ########## @@ -196,12 +196,35 @@ def _lock_reducer(obj): def dump_session(file_path): - # It is possible to dump session with cloudpickle. However, since references - # are saved it should not be necessary. See https://s.apache.org/beam-picklers - pass + # Since References are saved (https://s.apache.org/beam-picklers), we only + # dump supported Beam Registries (currently only logical type registry) + from apache_beam.typehints import schemas + from apache_beam.coders import typecoders Review Comment: That's true. It doesn't have error, meaning "worker doesnt actually need the coder registry" in this use case. The original error is due to logical type registry, adding coder registry here is for completeness coneptually as both are exposed to user. ########## sdks/python/apache_beam/internal/cloudpickle_pickler.py: ########## @@ -196,12 +196,35 @@ def _lock_reducer(obj): def dump_session(file_path): - # It is possible to dump session with cloudpickle. However, since references - # are saved it should not be necessary. See https://s.apache.org/beam-picklers - pass + # Since References are saved (https://s.apache.org/beam-picklers), we only + # dump supported Beam Registries (currently only logical type registry) + from apache_beam.typehints import schemas + from apache_beam.coders import typecoders Review Comment: That's true. It doesn't have error, meaning "worker doesnt actually need the coder registry" in this use case. The original error is due to logical type registry, adding coder registry here is for completeness conceptually as both are exposed to user. -- 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]
