[GitHub] [flink] TisonKun commented on a change in pull request #7314: [FLINK-11135][YARN] Refactor Hadoop config loading in HadoopUtils

[GitBox <gi...@apache.org>]

TisonKun commented on a change in pull request #7314: [FLINK-11135][YARN] Refactor Hadoop config loading in HadoopUtils
URL: https://github.com/apache/flink/pull/7314#discussion_r363639998
 
 

 ##########
 File path: flink-filesystems/flink-hadoop-fs/src/test/java/org/apache/flink/runtime/fs/hdfs/HadoopConfigLoadingTest.java
 ##########
 @@ -180,15 +180,111 @@ public void loadFromEnvVariables() throws Exception {
 		assertEquals(IN_CP_CONFIG_VALUE, hadoopConf.get(IN_CP_CONFIG_KEY, null));
 	}
 
+	@Test
+	public void loadOverlappingConfig() throws Exception {
+		final String k1 = "key1";
+		final String k2 = "key2";
+		final String k3 = "key3";
+		final String k4 = "key4";
+		final String k5 = "key5";
+
+		final String v1 = "from HADOOP_CONF_DIR";
+		final String v2 = "from Flink config `fs.hdfs.hadoopconf`";
+		final String v3 = "from Flink config `fs.hdfs.hdfsdefault`";
+		final String v4 = "from HADOOP_HOME/etc/hadoop";
+		final String v5 = "from HADOOP_HOME/conf";
+
+		final File hadoopConfDir = tempFolder.newFolder("hadoopConfDir");
+		final File hadoopConfEntryDir = tempFolder.newFolder("hadoopConfEntryDir");
+		final File legacyConfDir = tempFolder.newFolder("legacyConfDir");
+		final File hadoopHome = tempFolder.newFolder("hadoopHome");
+
+		final File hadoopHomeConf = new File(hadoopHome, "conf");
+		final File hadoopHomeEtc = new File(hadoopHome, "etc/hadoop");
+
+		assertTrue(hadoopHomeConf.mkdirs());
+		assertTrue(hadoopHomeEtc.mkdirs());
+
+		final File file1 = new File(hadoopConfDir, "core-site.xml");
+		final File file2 = new File(hadoopConfEntryDir, "core-site.xml");
+		final File file3 = new File(legacyConfDir, "core-site.xml");
+		final File file4 = new File(hadoopHomeEtc, "core-site.xml");
+		final File file5 = new File(hadoopHomeConf, "core-site.xml");
+
+		printConfig(file1, k1, v1);
+
+		Map<String, String> properties2 = new HashMap<>();
+		properties2.put(k1, v2);
+		properties2.put(k2, v2);
+		printConfigs(file2, properties2);
+
+		Map<String, String> properties3 = new HashMap<>();
+		properties3.put(k1, v3);
+		properties3.put(k2, v3);
+		properties3.put(k3, v3);
+		printConfigs(file3, properties3);
+
+		Map<String, String> properties4 = new HashMap<>();
+		properties4.put(k1, v4);
+		properties4.put(k2, v4);
+		properties4.put(k3, v4);
+		properties4.put(k4, v4);
+		printConfigs(file4, properties4);
+
+		Map<String, String> properties5 = new HashMap<>();
+		properties5.put(k1, v5);
+		properties5.put(k2, v5);
+		properties5.put(k3, v5);
+		properties5.put(k4, v5);
+		properties5.put(k5, v5);
+		printConfigs(file5, properties5);
+
+		final Configuration cfg = new Configuration();
+		cfg.setString(ConfigConstants.PATH_HADOOP_CONFIG, hadoopConfEntryDir.getAbsolutePath());
+		cfg.setString(ConfigConstants.HDFS_DEFAULT_CONFIG, file3.getAbsolutePath());
+
+		final org.apache.hadoop.conf.Configuration hadoopConf;
+
+		final Map<String, String> originalEnv = System.getenv();
+		final Map<String, String> newEnv = new HashMap<>(originalEnv);
+		newEnv.put("HADOOP_CONF_DIR", hadoopConfDir.getAbsolutePath());
+		newEnv.put("HADOOP_HOME", hadoopHome.getAbsolutePath());
+		try {
+			CommonTestUtils.setEnv(newEnv);
+			hadoopConf = HadoopUtils.getHadoopConfiguration(cfg);
+		}
+		finally {
+			CommonTestUtils.setEnv(originalEnv);
+		}
+
+		// contains extra entries
+		assertEquals(v1, hadoopConf.get(k1, null));
 
 Review comment:
   How these assertions check the correct fallback path? Possibly we need different value of the same key to check the correct priority.

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


With regards,
Apache Git Services