Abacn commented on code in PR #36271: URL: https://github.com/apache/beam/pull/36271#discussion_r2392446805
########## CHANGES.md: ########## @@ -94,6 +94,10 @@ * PulsarIO has now changed support status from incomplete to experimental. Both read and writes should now minimally function (un-partitioned topics, without schema support, timestamp ordered messages for read) (Java) ([#36141](https://github.com/apache/beam/issues/36141)). +* (Python) Logical type and coder registry are saved for pipelines with `save_main_session` pipeline option enabled in Review Comment: Well there are two use case here * User set `--save_main_session` in Beam 2.64.0 and relies on it (for registries). This should still work in newer version. This PR fixes this case. * User set `--save_main_session` in Beam 2.64.0 and no longer relies on it (e.g. for user defined classes). This change won't break them, as registries are merged on load. Yes it is no longer "no-op" but expected to stay no effect as long as remote SDK versions are expected to be in sync. If it unexpectedly having error it's due to some other unexposed bugs, and users should then remove `--save_main_session` flag because this flag was "unintentional" ------- > can we detect that someone registered a logical type It's hard. logical type registry is used to not only adding new types but also to **overwrite** the mapping of those defined in schema.py (e.g. the notorious MillisInstant) -- 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]
