chamikaramj commented on a change in pull request #15857:
URL: https://github.com/apache/beam/pull/15857#discussion_r746901926



##########
File path: sdks/python/apache_beam/utils/subprocess_server.py
##########
@@ -283,6 +290,39 @@ def beam_services(cls, replacements):
     finally:
       cls._BEAM_SERVICES.replacements = old
 
+  @classmethod
+  def make_classpath_jar(cls, main_jar, extra_jars, cache_dir=None):

Review comment:
       Please add a unit test for this as well.

##########
File path: sdks/python/apache_beam/utils/subprocess_server.py
##########
@@ -283,6 +290,39 @@ def beam_services(cls, replacements):
     finally:
       cls._BEAM_SERVICES.replacements = old
 
+  @classmethod
+  def make_classpath_jar(cls, main_jar, extra_jars, cache_dir=None):
+    if cache_dir is None:
+      cache_dir = cls.JAR_CACHE
+    composite_jar_dir = os.path.join(cache_dir, 'composite-jars')
+    os.makedirs(composite_jar_dir, exist_ok=True)
+    classpath = []
+    # Class-Path references from a jar must be relative.
+    for pattern in [main_jar] + list(extra_jars):
+      for path in glob.glob(pattern) or [pattern]:
+        rel_path = hashlib.sha256(
+            path.encode('utf-8')).hexdigest() + os.path.splitext(path)[1]
+        classpath.append(rel_path)
+        if not os.path.lexists(os.path.join(composite_jar_dir, rel_path)):
+          os.symlink(path, os.path.join(composite_jar_dir, rel_path))
+    composite_jar = os.path.join(
+        composite_jar_dir,
+        hashlib.sha256(' '.join(sorted(classpath)).encode('ascii')).hexdigest()
+        + '.jar')
+    if not os.path.exists(composite_jar):

Review comment:
       It's unclear to me if this will work for all Jars. @lukecwik might be 
able to confirm. (non-blocking comment)

##########
File path: sdks/python/apache_beam/utils/subprocess_server.py
##########
@@ -283,6 +290,39 @@ def beam_services(cls, replacements):
     finally:
       cls._BEAM_SERVICES.replacements = old
 
+  @classmethod
+  def make_classpath_jar(cls, main_jar, extra_jars, cache_dir=None):
+    if cache_dir is None:
+      cache_dir = cls.JAR_CACHE
+    composite_jar_dir = os.path.join(cache_dir, 'composite-jars')
+    os.makedirs(composite_jar_dir, exist_ok=True)
+    classpath = []
+    # Class-Path references from a jar must be relative.

Review comment:
       Could you expand the comment here to explain what's done here.

##########
File path: sdks/python/apache_beam/utils/subprocess_server.py
##########
@@ -158,7 +161,11 @@ class JavaJarServer(SubprocessServer):
       'local', (threading.local, ),
       dict(__init__=lambda self: setattr(self, 'replacements', {})))()
 
-  def __init__(self, stub_class, path_to_jar, java_arguments):
+  def __init__(self, stub_class, path_to_jar, java_arguments, classpath=None):
+    if classpath or True:

Review comment:
       This is always True ?




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to