You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/08/09 05:16:01 UTC
[GitHub] [flink] zenfenan commented on a change in pull request #12823: FLINK-18013: Refactor Hadoop utils to a single module
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:
> 
>
> 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:
users@infra.apache.org