bgeng777 commented on a change in pull request #18531:
URL: https://github.com/apache/flink/pull/18531#discussion_r801334847
##########
File path:
flink-yarn/src/main/java/org/apache/flink/yarn/YarnApplicationFileUploader.java
##########
@@ -335,7 +335,7 @@ public YarnLocalResourceDescriptor uploadFlinkDist(final
Path localJarPath)
*
* @return list of class paths with the file name
*/
- List<String> registerProvidedLocalResources() {
+ List<String> registerProvidedLocalResources(boolean
shouldUsrLibExistInRemote) {
Review comment:
Yes, I totally agree with the assumption.
After reading your following comments about
`isUsrLibDirIncludedInProvidedLib()`, I now prefer to add `checkArgument` of
`providedLibDirs()` in the constructor.
My only concern is that is it a good idea to introduce
`CLASSPATH_INCLUDE_USER_JAR` into YarnApplicationFileUploader? It seems adding
unnecessary coupling but without this config, we cannot to check the case that
`CLASSPATH_INCLUDE_USER_JAR` is `DISABLED` while there is `usrlib` in
providedLibDirs. Such case is allowed for ship files but for providedLibDirs,
my opinion is directly not to allow such case. What's your opinion?
--
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
