This is an automated email from the ASF dual-hosted git repository. janardhan pushed a commit to branch py-logging-issue in repository https://gitbox.apache.org/repos/asf/systemds.git
commit e4b4546e6bcbf17b7612c35765f2fd47d19e90bc Author: Sebastian Baunsgaard <[email protected]> AuthorDate: Thu Apr 4 19:05:19 2024 +0200 [SYSTEMDS-3687] Python API startup fixes This PR change the startup of the python interface to properly use the jar files, and fixes a release issue where if the SYSTEMDS_ROOT is not set the python API did not properly hook into the released jar artifacts. --- .gitignore | 1 - src/main/python/.gitignore | 7 +++ .../python/{.gitignore => conf/log4j.properties} | 37 +++++-------- src/main/python/pre_setup.py | 10 ++++ .../python/systemds/context/systemds_context.py | 61 +++++++++++++++------- 5 files changed, 71 insertions(+), 45 deletions(-) diff --git a/.gitignore b/.gitignore index 1a83a3a80e..5e40b37fd9 100644 --- a/.gitignore +++ b/.gitignore @@ -64,7 +64,6 @@ buildNumber.properties _site/ # Tutorial data mnist -src/main/python/systemds/examples/tutorials/*/ scripts/nn/examples/data/* # User configuration files diff --git a/src/main/python/.gitignore b/src/main/python/.gitignore index ea43a07f92..0ac4a56899 100644 --- a/src/main/python/.gitignore +++ b/src/main/python/.gitignore @@ -44,3 +44,10 @@ tests/examples/tutorials/model tests/lineage/temp python_venv/ +venv + +# Main Jar location for API communiation. +systemds/SystemDS.jar + +# tutorial +systemds/examples/tutorials/* diff --git a/src/main/python/.gitignore b/src/main/python/conf/log4j.properties similarity index 64% copy from src/main/python/.gitignore copy to src/main/python/conf/log4j.properties index ea43a07f92..9a381e00aa 100644 --- a/src/main/python/.gitignore +++ b/src/main/python/conf/log4j.properties @@ -7,9 +7,9 @@ # to you under the Apache License, Version 2.0 (the # "License"); you may not use this file except in compliance # with the License. You may obtain a copy of the License at -# +# # http://www.apache.org/licenses/LICENSE-2.0 -# +# # Unless required by applicable law or agreed to in writing, # software distributed under the License is distributed on an # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -19,28 +19,15 @@ # #------------------------------------------------------------- -# Git ignore for python files. -systemds/lib/ -systemds.egg-info/ -systemds/conf/ -build/ -LICENSE -NOTICE -generator.log -dist -docs/build -docs/source/_build -tests/onnx_systemds/output_test -tests/onnx_systemds/dml_output -tests/onnx_systemds/test_models/*.onnx +log4j.rootLogger=ERROR,console -# git ignore tmp test files -tests/federated/output -tests/federated/worker -tests/federated/tmp -tests/list/tmp -tests/algorithms/readwrite/ -tests/examples/tutorials/model -tests/lineage/temp +log4j.logger.org.apache.sysds=ERROR +log4j.logger.org.apache.sysds.utils.SettingsChecker=WARN +log4j.logger.org.apache.spark=ERROR +log4j.logger.org.apache.hadoop=OFF +log4j.logger.org.apache.hadoop.util.NativeCodeLoader=INFO -python_venv/ +log4j.appender.console=org.apache.log4j.ConsoleAppender +log4j.appender.console.target=System.err +log4j.appender.console.layout=org.apache.log4j.PatternLayout +log4j.appender.console.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p %c{2}: %m%n diff --git a/src/main/python/pre_setup.py b/src/main/python/pre_setup.py index 5c7314a4e7..e4bb99ae3f 100755 --- a/src/main/python/pre_setup.py +++ b/src/main/python/pre_setup.py @@ -80,6 +80,16 @@ if not os.path.exists(CONF_DIR): shutil.copy(os.path.join(SYSTEMDS_ROOT,'conf', 'log4j.properties'), os.path.join(this_path, PYTHON_DIR, 'conf', 'log4j.properties')) shutil.copy(os.path.join(SYSTEMDS_ROOT,'conf', 'SystemDS-config-defaults.xml'), os.path.join(this_path, PYTHON_DIR, 'conf', 'SystemDS-config-defaults.xml')) + +# Take SystemDS file. +shutil.copy(os.path.join(SYSTEMDS_ROOT, 'target', 'SystemDS.jar'), os.path.join(PYTHON_DIR, 'SystemDS.jar')) + +# remove redundant SystemDS file inside lib. +for file in os.listdir(os.path.join(PYTHON_DIR, 'lib')): + if "systemds" in file: + if "extra" not in file: + os.remove(os.path.join(PYTHON_DIR, 'lib', file)) + SYSTEMDS_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.getcwd()))) shutil.copyfile(os.path.join(SYSTEMDS_ROOT, 'LICENSE'), 'LICENSE') shutil.copyfile(os.path.join(SYSTEMDS_ROOT, 'NOTICE'), 'NOTICE') diff --git a/src/main/python/systemds/context/systemds_context.py b/src/main/python/systemds/context/systemds_context.py index 5f34086807..85d2588c64 100644 --- a/src/main/python/systemds/context/systemds_context.py +++ b/src/main/python/systemds/context/systemds_context.py @@ -155,34 +155,47 @@ class SystemDSContext(object): """Build the command line argument for the startup of the JVM :param port: The port address to use if -1 chose random port.""" + # Base command command = ["java", "-cp"] + + # Find the operating system specifc separator, nt means its Windows + cp_separator = ";" if os.name == "nt" else ":" root = os.environ.get("SYSTEMDS_ROOT") + if root == None: - # If there is no systemds install default to use the PIP packaged java files. + # If there is no systemds install default to use the pip packaged java files. root = os.path.join(get_module_dir()) - # nt means its Windows - cp_separator = ";" if os.name == "nt" else ":" - - if os.environ.get("SYSTEMDS_ROOT") != None: + # Find the SystemDS jar file. + if root != None: # root path was set + self._log.debug("SYSTEMDS_ROOT was set, searching for jar file") lib_release = os.path.join(root, "lib") - lib_cp = os.path.join(root, "target", "lib") - if os.path.exists(lib_release): - classpath = cp_separator.join([os.path.join(lib_release, '*')]) - elif os.path.exists(lib_cp): - systemds_cp = os.path.join(root, "target", "SystemDS.jar") - classpath = cp_separator.join( - [os.path.join(lib_cp, '*'), systemds_cp]) + systemds_cp = os.path.join(root, "target", "SystemDS.jar") + if os.path.exists(lib_release): # It looks like it was a release path for root. + classpath = os.path.join(root, "SystemDS.jar") + if not os.path.exists(classpath): + for f in os.listdir(root): + if "systemds" in f: + if os.path.exists(classpath): + raise(ValueError("Invalid setup there were multiple conflicting systemds jar fines in" + root)) + else: + classpath = os.path.join(root, f) + if not os.path.exists(classpath): + raise ValueError( + "Invalid setup did not find SystemDS jar file in " + root) + elif os.path.exists(systemds_cp): + classpath = cp_separator.join([systemds_cp]) else: raise ValueError( - "Invalid setup at SYSTEMDS_ROOT env variable path " + lib_cp) - else: - lib1 = os.path.join(root, "lib", "*") - lib2 = os.path.join(root, "lib") - classpath = cp_separator.join([lib1, lib2]) + "Invalid setup at SYSTEMDS_ROOT env variable path " + root) + else: # root path was not set use the pip installed SystemDS + self._log.warning("SYSTEMDS_ROOT was unset, defaulting to python packaged jar files") + systemds_cp = os.path.join(root,"SystemDS.jar") + classpath = cp_separator.join([systemds_cp]) command.append(classpath) + # Find the logging configuration file. if os.environ.get("LOG4JPROP") == None: files = glob(os.path.join(root, "conf", "log4j*.properties")) if len(files) > 1: @@ -195,16 +208,23 @@ class SystemDSContext(object): else: command.append("-Dlog4j.configuration=file:" + files[0]) else: - command.append("-Dlog4j.configuration=file:" +os.environ.get("LOG4JPROP")) + logging_file = os.environ.get("LOG4JPROP") + if os.path.exists(logging_file): + command.append("-Dlog4j.configuration=file:" +os.environ.get("LOG4JPROP")) + else: + self._log.warning("LOG4JPROP is set but path is invalid: " + str(logging_file)) + # Specify the main function inside SystemDS to launch in java. command.append("org.apache.sysds.api.PythonDMLScript") + # Find the configuration file for systemds. + # TODO: refine the choise of configuration file files = glob(os.path.join(root, "conf", "SystemDS*.xml")) if len(files) > 1: self._log.warning( "Multiple config files found selecting: " + files[0]) if len(files) == 0: - self._log.warning("No log4j file found at: " + self._log.warning("No xml config file found at: " + os.path.join(root, "conf") + " therefore using default settings") else: @@ -219,6 +239,9 @@ class SystemDSContext(object): command.append("--python") command.append(str(actual_port)) + self._log.info("Command " + str(command)) + self._log.info("Port used for communication: " + str(actual_port)) + return command, actual_port def __start(self, port: int, capture_stdout: bool, retry: int = 0):
