TisonKun edited a comment on issue #11472: [FLINK-16547][yarn] Respect the config option of FileJobGraphRetriever#JOB_GRAPH_FILE_PATH URL: https://github.com/apache/flink/pull/11472#issuecomment-602458510 > Thanks for the work @zhengcanbin and for the review @TisonKun. I will merge this. > > Side note for a potential future JIRA: in the `startAppMaster()` method we are using sometime the `configuration` argument to the method to get/set config options, and sometime the `flinkConfiguration` which is a class member. Shouldn't we always use the `configuration` argument? This will make the method more self-contained. @kl0u yes I think I get some cycle of that problem already. However, I'd prefer refer to the field `flinkConfiguration`; otherwise user(previous me) might ask what's the difference between *this configuration object* and *the one I've seen as field*? It is one of Java best practices that we don't pass field as parameter of this class's method. Besides, I've heard developers complain with the tight couplings between `YarnClusterDescriptor` and `Utils`. There are several huge & context-aware function. If YARN deployment is still under active development, we might think of refactor these codes gradually. cc @wangyang0918
---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services
