bgeng777 commented on a change in pull request #18531: URL: https://github.com/apache/flink/pull/18531#discussion_r802311817
########## File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnClusterDescriptorTest.java ########## @@ -699,6 +701,32 @@ public void testDisableSystemClassPathIncludeUserJarAndWithIllegalShipDirectoryN } } + /** Tests that the usrlib will be automatically shipped. */ + @Test + public void testShipUsrLib() throws IOException { + final Map<String, String> oldEnv = System.getenv(); + final Map<String, String> env = new HashMap<>(1); + File homeFolder = temporaryFolder.newFolder().getAbsoluteFile(); + File libFolder = new File(homeFolder.getAbsolutePath(), "lib"); + assertTrue(libFolder.createNewFile()); + File usrLibFolder = + new File(homeFolder.getAbsolutePath(), ConfigConstants.DEFAULT_FLINK_USR_LIB_DIR); + assertTrue(usrLibFolder.mkdirs()); + File usrLibFile = new File(usrLibFolder, "usrLibFile.jar"); + assertTrue(usrLibFile.createNewFile()); + env.put(ConfigConstants.ENV_FLINK_LIB_DIR, libFolder.getAbsolutePath()); + CommonTestUtils.setEnv(env); + + try (YarnClusterDescriptor descriptor = createYarnClusterDescriptor()) { + Set<File> effectiveShipFiles = new HashSet<>(); + descriptor.addUsrLibFolderToShipFiles(effectiveShipFiles); + assertThat(effectiveShipFiles, containsInAnyOrder(usrLibFolder)); + assertThat(effectiveShipFiles, not(containsInAnyOrder(usrLibFile))); Review comment: The `assertThat(effectiveShipFiles, not(containsInAnyOrder(usrLibFile)));` just follows the check logic in testEnvironmentDirectoryShipping to make sure single is not added. ########## File path: flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java ########## @@ -918,6 +915,18 @@ private ApplicationReport startAppMaster( : Path.CUR_DIR, LocalResourceType.FILE); + // usrlib will be automatically shipped if it exists. + final Set<File> usrLibShipFiles = new HashSet<>(); Review comment: I remove this check because I double checked the codes and it should work well as we will do this check in addUsrLibFolderToShipFiles. For readability and less object allocation/function call , I agree to get it back. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org