[ 
https://issues.apache.org/jira/browse/FLINK-10304?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16607335#comment-16607335
 ] 

ASF GitHub Bot commented on FLINK-10304:
----------------------------------------

TisonKun commented on a change in pull request #6673: [FLINK-10304] [client] 
Remove deprecated AbstractYarnClusterDescriptor field
URL: https://github.com/apache/flink/pull/6673#discussion_r216017006
 
 

 ##########
 File path: 
flink-yarn/src/main/java/org/apache/flink/yarn/AbstractYarnClusterDescriptor.java
 ##########
 @@ -997,7 +980,6 @@ public ApplicationReport startAppMaster(
                appMasterEnv.put(YarnConfigKeys.ENV_CLIENT_HOME_DIR, 
homeDir.toString());
                appMasterEnv.put(YarnConfigKeys.ENV_CLIENT_SHIP_FILES, 
envShipFileList.toString());
                appMasterEnv.put(YarnConfigKeys.ENV_SLOTS, 
String.valueOf(clusterSpecification.getSlotsPerTaskManager()));
-               appMasterEnv.put(YarnConfigKeys.ENV_DETACHED, 
String.valueOf(detached));
 
 Review comment:
   I wonder if YARN need this config to distinguish a DETACHED session, if so, 
`AbstractYarnClusterDescriptor` need to know whether or not it is in a detached 
mode, and we should keep the original code and remove the `@Deprecated` 
annotation.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


> Remove deprecated AbstractYarnClusterDescriptor field
> -----------------------------------------------------
>
>                 Key: FLINK-10304
>                 URL: https://issues.apache.org/jira/browse/FLINK-10304
>             Project: Flink
>          Issue Type: Improvement
>          Components: Client, YARN
>    Affects Versions: 1.7.0
>            Reporter: 陈梓立
>            Assignee: 陈梓立
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.7.0
>
>
> Depend on [~trohrm...@apache.org]'s 
> [commit|https://github.com/apache/flink/commit/6356128865bff7463bf03185d18b129ed3633bc2],
>  {{AbstractYarnClusterDescriptor}} should not care whether it is in DETACHED 
> mode.
> After digging I found the main usages of it are
> 1. {{FlinkYarnSessionCli#run}}, this can be resolved by checking whether 
> {{allOptions}} has {{DETACHED_OPTION}} locally.
> 2. when AbstractYarnClusterDescriptor start a AM, it sets 
> {{appMasterEnv.put(YarnConfigKeys.ENV_DETACHED, String.valueOf(detached));}}. 
> At this point it seems that YarnClusterDescriptor should know whether or not 
> it is in detached mode.
> If usage 2 is irrelevant now, we can get rid of deprecated method in FLIP-6 
> codebase.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to