venkata91 commented on code in PR #23164:
URL: https://github.com/apache/flink/pull/23164#discussion_r1297514150
##########
flink-yarn/src/main/java/org/apache/flink/yarn/YarnApplicationFileUploader.java:
##########
@@ -360,11 +362,27 @@ List<String> registerProvidedLocalResources() {
envShipResourceList.add(descriptor);
if (!isFlinkDistJar(filePath.getName()) &&
!isPlugin(filePath)) {
- classPaths.add(fileName);
+ URI parentDirectoryUri = new
Path(fileName).getParent().toUri();
+ String relativeParentDirectory =
+ new Path(filePath.getName())
+ .toUri()
+ .relativize(parentDirectoryUri)
+ .toString();
+
+ if (!resourcesDir.contains(relativeParentDirectory)) {
+ resourcesDir.add(relativeParentDirectory);
+ }
+ resources.add(fileName);
Review Comment:
One thing to add here is, we are already `relativizing` the path so there
are no redundant top level directory names added to the classpath. For eg:
`/users/test/lib/hadoop/;/users/test/lib/hive` - `.:hadoop:hive:` will be added
to the classpath.
Note: the same applies to existing `yarn.ship-files` config as well which is
working fine without issues.
> Also, in general, adding the resource dir to class path in a more explicit
and accurate way should probably be preferred.
Do you mean resources as anything other than `jar` like `.xml`, `.yaml` etc
should be added through `yarn.provided.resources.dirs`? Can you please explain
what do you mean when you say `more explicit and accurate way`?
2 things:
1. Given this separation, won't the same issue still happen for `*.jar`?
2. I'm not super sure about introducing an additional config for this case.
--
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
