[ 
https://issues.apache.org/jira/browse/BEAM-8113?focusedWorklogId=305491&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-305491
 ]

ASF GitHub Bot logged work on BEAM-8113:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 03/Sep/19 09:29
            Start Date: 03/Sep/19 09:29
    Worklog Time Spent: 10m 
      Work Description: je-ik commented on pull request #9451: [BEAM-8113] 
Stage files from context classloader
URL: https://github.com/apache/beam/pull/9451#discussion_r320176839
 
 

 ##########
 File path: 
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/PipelineResources.java
 ##########
 @@ -40,34 +43,28 @@
 public class PipelineResources {
 
   /**
-   * Attempts to detect all the resources the class loader has access to. This 
does not recurse to
-   * class loader parents stopping it from pulling in resources from the 
system class loader.
+   * Detects all URLs that are present in all class loaders in between context 
class loader of
+   * calling thread and class loader of class passed as parameter. It doesn't 
follow parents above
+   * this class loader stopping it from pulling in resources from the system 
class loader.
    *
-   * @param classLoader The URLClassLoader to use to detect resources to stage.
-   * @throws IllegalArgumentException If either the class loader is not a 
URLClassLoader or one of
-   *     the resources the class loader exposes is not a file resource.
+   * @param cls Class whose class loader stops recursion into parent loaders
+   * @throws IllegalArgumentException no classloader in context hierarchy is 
URLClassloader or if
+   *     one of the resources any class loader exposes is not a file resource.
    * @return A list of absolute paths to the resources the class loader uses.
    */
-  public static List<String> detectClassPathResourcesToStage(ClassLoader 
classLoader) {
-    if (!(classLoader instanceof URLClassLoader)) {
-      String message =
-          String.format(
-              "Unable to use ClassLoader to detect classpath elements. "
-                  + "Current ClassLoader is %s, only URLClassLoaders are 
supported.",
-              classLoader);
-      throw new IllegalArgumentException(message);
+  public static List<String> detectClassPathResourcesToStage(Class<?> cls) {
+    Set<String> files = new HashSet<>();
+    ClassLoader stoppingLoader = cls.getClassLoader();
+    ClassLoader currentLoader = ReflectHelpers.findClassLoader();
+    while (currentLoader != null && stoppingLoader != currentLoader) {
+      files.addAll(extractResourcesFromClassLoader(currentLoader));
+      currentLoader = currentLoader.getParent();
     }
-
-    List<String> files = new ArrayList<>();
-    for (URL url : ((URLClassLoader) classLoader).getURLs()) {
-      try {
-        files.add(new File(url.toURI()).getAbsolutePath());
-      } catch (IllegalArgumentException | URISyntaxException e) {
-        String message = String.format("Unable to convert url (%s) to file.", 
url);
-        throw new IllegalArgumentException(message, e);
-      }
+    files.addAll(extractResourcesFromClassLoader(currentLoader));
+    if (files.isEmpty()) {
+      throw new IllegalArgumentException("Unable to use ClassLoader to detect 
classpath elements.");
 
 Review comment:
   Actually, there is a test in `PipelineResourcesTest`, that explicitly tests 
this behavior. And I think it is fine, because otherwise we would again 
introduce breaking change for JDK >= 9, because we would just silently skip all 
classloaders and users would get weird exceptions at pipeline runtime (although 
it have worked on JDK8). I think it is better to let users know about that in a 
way that is more understandable.
 
----------------------------------------------------------------
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


Issue Time Tracking
-------------------

    Worklog Id:     (was: 305491)
    Time Spent: 4h  (was: 3h 50m)

> FlinkRunner: Stage files from context classloader
> -------------------------------------------------
>
>                 Key: BEAM-8113
>                 URL: https://issues.apache.org/jira/browse/BEAM-8113
>             Project: Beam
>          Issue Type: Improvement
>          Components: runner-flink
>            Reporter: Jan Lukavský
>            Assignee: Jan Lukavský
>            Priority: Major
>          Time Spent: 4h
>  Remaining Estimate: 0h
>
> Currently, only files from {{FlinkRunner.class.getClassLoader()}} are staged 
> by default. Add also files from 
> {{Thread.currentThread().getContextClassLoader()}}.



--
This message was sent by Atlassian Jira
(v8.3.2#803003)

Reply via email to