robertwb commented on code in PR #33432:
URL: https://github.com/apache/beam/pull/33432#discussion_r1894331690


##########
sdks/python/apache_beam/coders/coders.py:
##########
@@ -1039,11 +1039,16 @@ def __hash__(self):
 
   @classmethod
   def from_type_hint(cls, typehint, unused_registry):
-    if issubclass(typehint, proto_utils.message_types):
+    # The typehint must be a subclass of google.protobuf.message.Message.
+    # ProtoCoder cannot work with message.Message itself, as required APIs are
+    # not implemented in the base class. If this occurs, an error is raised

Review Comment:
   Perhaps clarify the issue a bit more, e.g.
   
   ProtoCoder cannot work with message.Message itself, as deserialization of a 
serialized proto requires knowledge of the desired concrete proto subclass 
which is not stored in the encoded bytes themselves.



##########
sdks/python/apache_beam/coders/coders_test.py:
##########
@@ -86,6 +87,23 @@ def test_proto_coder(self):
     self.assertEqual(ma, real_coder.decode(real_coder.encode(ma)))
     self.assertEqual(ma.__class__, real_coder.to_type_hint())
 
+  def test_proto_coder_on_protobuf_message_subclasses(self):
+    # This replicates a scenario where users provide message.Message as the
+    # output typehint for a Map function, even though the actual output 
messages
+    # are subclasses of message.Message.
+    ma = test_message.MessageA()
+    mb = ma.field2.add()
+    mb.field1 = True
+    ma.field1 = 'hello world'
+
+    coder = coders_registry.get_coder(message.Message)
+    # For messages of google.protobuf.message.Message, the fallback coder will
+    # be FastPrimitiveCoder other than ProtoCoder.

Review Comment:
   ...rather than ProtoCoder.



##########
sdks/python/apache_beam/coders/coders.py:
##########
@@ -1039,11 +1039,16 @@ def __hash__(self):
 
   @classmethod
   def from_type_hint(cls, typehint, unused_registry):
-    if issubclass(typehint, proto_utils.message_types):
+    # The typehint must be a subclass of google.protobuf.message.Message.

Review Comment:
   ...a strict subclass...



-- 
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]

Reply via email to