flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] zjffdu commented on a change in pull request #8144: [FLINK-12159]. Enable YarnMiniCluster integration test under non-secure mode
Date Sun, 05 May 2019 06:23:21 GMT
zjffdu commented on a change in pull request #8144: [FLINK-12159]. Enable YarnMiniCluster integration
test under non-secure mode
URL: https://github.com/apache/flink/pull/8144#discussion_r281006208
 
 

 ##########
 File path: flink-yarn/src/main/java/org/apache/flink/yarn/AbstractYarnClusterDescriptor.java
 ##########
 @@ -869,41 +859,40 @@ public ApplicationReport startAppMaster(
 		FsPermission permission = new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.NONE);
 		fs.setPermission(yarnFilesDir, permission); // set permission for path.
 
+		Path remoteYarnSiteXmlPath = null;
+		File f = new File(System.getenv("YARN_CONF_DIR"), Utils.YARN_SITE_FILE_NAME);
+		LOG.info("Adding Yarn configuration {} to the AM container local resource bucket", f.getAbsolutePath());
+		Path yarnSitePath = new Path(f.getAbsolutePath());
+		remoteYarnSiteXmlPath = setupSingleLocalResource(
+			Utils.YARN_SITE_FILE_NAME,
+			fs,
+			appId,
+			yarnSitePath,
+			localResources,
+			homeDir,
+			"");
 
 Review comment:
   > I guess one could then refactor the YarnTestBase#Runner to instantiate the FlinkYarnSessionCli
with a Supplier<YarnClusterDescriptor> or factory which could setup the YarnClusterDescriptor
accordingly.
   
   @tillrohrmann IMHO, this is not a good solution as well. Because it would also introduce
different code path for test code and production code in `FlinkYarnSessionCli`. 
   I totally agree that we need to refactor the flink client api, currently the command line
parsing and cluster deployment is coupled together closely, which make it difficult to do
unit testing (The test code in flink-yarn-tests module is not readable to me, which imply
the issues of current flink api design).  I do have a plan to refactor the flink client api,
but it would involve a large amount of work. 
   For this PR, it is just to fix the YarnMiniCluster integration test bug so that downstream
project can do integration test with flink. I think we can just fix the bug without touching
the client api (keep `IN_TESTS`) for now. The refactoring of flink client api could be done
in other PRs. Does it make sense to you ?
    
   
    

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

Mime
View raw message