chamikaramj commented on a change in pull request #15727:
URL: https://github.com/apache/beam/pull/15727#discussion_r731215579
##########
File path: sdks/python/apache_beam/io/gcp/pubsublite/external.py
##########
@@ -0,0 +1,119 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
Review comment:
This was mainly for consistency with existing I/O connectors.
Main points are:
* From a Python pipeline author's perspective there should not be a
difference between using a cross-language transform from a natively implemented
Python transform (in addition to the extra expansion_service parameter which
should be optional).
* Any transform that is not in the public API should preferably start with
"_".
* Usually for existing Python I/O connectors, maintaining a single module
with all required classes has been adequate (see modules in
https://github.com/apache/beam/tree/master/sdks/python/apache_beam/io/gcp)
I'm OK with defining a sub-module if needed but looking at the code changes
here I'm not sure if it's worth it. Are you expecting a significant amount of
future code changes ? Given that this these are cross-language transforms,
hopefully amount of additional logic needed in Python side will be minimal.
--
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]
