lgajowy commented on a change in pull request #12117:
URL: https://github.com/apache/beam/pull/12117#discussion_r451677939



##########
File path: 
sdks/java/io/snowflake/src/main/java/org/apache/beam/sdk/io/snowflake/services/SnowflakeServiceImpl.java
##########
@@ -25,7 +27,9 @@
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import javax.sql.DataSource;
+import org.apache.beam.sdk.io.snowflake.data.SnowflakeTableSchema;
 import org.apache.beam.sdk.io.snowflake.enums.CloudProvider;

Review comment:
       I'm not sure, to be honest. Users should look for that information in 
the docs. Note that technically (not that I'd wish for this) there are ways of 
providing support to other cloud providers without extending that enum so 
looking at the endpoint is not the ultimate proof that only gcs is supported. 
If there's no docs or it's out of date regarding cloud providers support, the 
devs will still need to dig into the IO implementation details to really find 
out. So having this enum public is not much help imho.
   
   On the other hand, what we do now is that we leave an implementation detail 
(noise) that is not useful for the users in any way in code in the public API. 
I'd suggest hiding it and maybe expose it later if there is a good reason to do 
so. :)




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to