zenfenan commented on a change in pull request #12823:
URL: https://github.com/apache/flink/pull/12823#discussion_r467537692



##########
File path: 
flink-hadoop-utils/src/main/java/org/apache/flink/hadoop/utils/HadoopUtils.java
##########
@@ -16,7 +16,7 @@
  * limitations under the License.
  */
 
-package org.apache.flink.runtime.util;
+package org.apache.flink.hadoop.utils;
 
 import org.apache.flink.api.java.tuple.Tuple2;
 import org.apache.flink.configuration.ConfigConstants;

Review comment:
       Makes sense. I feel it is better if we separate out the utility class at 
functionality level rather than having an uber HadoopUtils class. Would also 
avoid confusion. I will do that but do we have util classes for Hadoop security 
now to be moved under this new module?

##########
File path: 
flink-hadoop-utils/src/main/java/org/apache/flink/hadoop/utils/HadoopUtils.java
##########
@@ -16,7 +16,7 @@
  * limitations under the License.
  */
 
-package org.apache.flink.runtime.util;
+package org.apache.flink.hadoop.utils;
 
 import org.apache.flink.api.java.tuple.Tuple2;
 import org.apache.flink.configuration.ConfigConstants;

Review comment:
       > It seems that this change is affecting the maven dependency tree quite 
a bit.
   > For example, here:
   > 
![image](https://user-images.githubusercontent.com/89049/87952139-bf8a0a00-caa9-11ea-8a78-139b4e799033.png)
   > 
   > This is problematic, as it might modify Flink's own dependencies (in 
particular in modules connecting to external systems), and we might 
"accidentally" include dependencies in our dependency shading.
   > Maybe we can use a wildcard exclude for the Hadoop dependencies?
   > I was also wondering if we can rid of the `hadoop-hdfs` dependency. It 
only seems to be there because of `HdfsConfiguration`. Maybe we can work around 
that (by manually calling 
`Configuration.addDefaultResource("hdfs-default.xml"); 
Configuration.addDefaultResource("hdfs-site.xml");` ? )
   
   Aah.. Good that you caught I should have compared the dependency trees 
myself. I will work on getting this resolved.




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